abstractdog commented on code in PR #540:
URL: https://github.com/apache/tez/pull/540#discussion_r4060487407
##########
tez-runtime-library/src/test/java/org/apache/tez/runtime/library/common/shuffle/impl/TestShuffleManager.java:
##########
@@ -160,6 +161,77 @@ public void testMultiplePartitions() throws Exception {
shuffleManager.getNumOfCompletedInputs());
}
+ private long approximateInputRecords(InputContext inputContext) {
+ return
inputContext.getCounters().findCounter(TaskCounter.APPROXIMATE_INPUT_RECORDS).getValue();
+ }
Review Comment:
this method can go below the `@Test` methods
##########
tez-runtime-library/src/main/java/org/apache/tez/runtime/library/common/shuffle/impl/ShuffleManager.java:
##########
@@ -348,13 +352,27 @@ public ShuffleManager(InputContext inputContext,
Configuration conf, int numInpu
+ ", asyncHttp=" + asyncHttp);
}
- public void updateApproximateInputRecords(int delta) {
- if (delta <= 0) {
+ /**
+ * Takes the row count one input announces and extrapolates the total over
every input.
+ * An announcement is the source task's output record count so far, not a
per-event delta, so a
+ * pipelined input announces a new total per spill and only its latest one
is kept -- it is the
+ * count at the time the event was built, which the final event brings up to
date. An input that
+ * wrote no rows announces zero and still joins the denominator, which
numInputs already counts
+ * in the multiplier. A count the writer could not fit in an int arrives
negative; ignore it
+ * rather than subtract it from the total.
+ */
+ public void updateApproximateInputRecords(int inputIndex, int numRecords) {
Review Comment:
nit: this is kind of shuffle internals, a package-protected scope makes more
sense to me
I can seen it used to be public, but as the signature is already touched by
this PR, maybe make it happen now
##########
tez-runtime-library/src/test/java/org/apache/tez/runtime/library/common/shuffle/impl/TestShuffleManager.java:
##########
Review Comment:
consider adding a unit test case for negative `numRecords`
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
