Updated based on feedback from Hong. Changed the checks to be per-task rather than looking at the whole job. Tolerances are proportional to the number of map/reduce tasks for bytes, 1 record for input/output.

Also tightened the reduce output bytes and improved reporting for errors during startup.

Chris Douglas
added a comment - 08/Oct/09 16:32 Updated based on feedback from Hong. Changed the checks to be per-task rather than looking at the whole job. Tolerances are proportional to the number of map/reduce tasks for bytes, 1 record for input/output.
Also tightened the reduce output bytes and improved reporting for errors during startup.

Patch looks good. The detailed verification is rigorous, which is nice.

One minor nit: why do we need to set the actual extra bytes and records proportional to nMaps and nReds. Does it make sense that for map output, each map would output at most extra max(1, nReduce/nMaps) records? And for each reducer only one extra record?

Hong Tang
added a comment - 10/Oct/09 01:51 Patch looks good. The detailed verification is rigorous, which is nice.
One minor nit: why do we need to set the actual extra bytes and records proportional to nMaps and nReds. Does it make sense that for map output, each map would output at most extra max(1, nReduce/nMaps) records? And for each reducer only one extra record?

why do we need to set the actual extra bytes and records proportional to nMaps and nReds

If the spec expects 0 bytes/records, then the necessary spec data for each reduce needs to be forgiven. The amount of extra data will be proportional to the number of maps/reduces.

However, this is adjacent to some sloppiness in the map output, where the spec data is not written as part of the output, but rather as overhead. While the special case will still exist, right now it's the case for all jobs. Since the test still needs to tolerate the 0 cases, I was planning to tighten up the shuffle in a separate issue.

Chris Douglas
added a comment - 10/Oct/09 06:53 Thanks for the review
why do we need to set the actual extra bytes and records proportional to nMaps and nReds
If the spec expects 0 bytes/records, then the necessary spec data for each reduce needs to be forgiven. The amount of extra data will be proportional to the number of maps/reduces.
However, this is adjacent to some sloppiness in the map output, where the spec data is not written as part of the output, but rather as overhead. While the special case will still exist, right now it's the case for all jobs. Since the test still needs to tolerate the 0 cases, I was planning to tighten up the shuffle in a separate issue.