You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@tez.apache.org by "deniskuzZ (via GitHub)" <gi...@apache.org> on 2026/09/19 11:52:06 UTC
[PR] TEZ-4757: APPROXIMATE_INPUT_RECORDS over-counts the rows an input will deliver [tez]
deniskuzZ opened a new pull request, #540:
URL: https://github.com/apache/tez/pull/540
- An announcement carries the source task's output record count so far, not a per-event delta; keep the latest value per input instead of summing events, so a pipelined input's earlier spills are not counted again
- Count an input that wrote no rows in the denominator; leaving it out while numInputs keeps it in the multiplier spread the mean of the inputs that had data over the ones that did not
- Announce from the composite event path too: which path runs depends only on tez.am.shuffle.auxiliary-service.id, so on the Tez shuffle handler the counter was never updated at all
- Multiply before dividing, the mean was truncated first
- Saturate the announced count instead of wrapping it through int, and include large records, which bypass OUTPUT_RECORDS -- the VertexManager event already counts both
- updateApproximateInputRecords takes the input index
- Measured before the fix: 10 inputs, 2 announcing 1000 rows each, read 10,000 instead of 2,000; 4 pipelined events totalling 600 read 450
--
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: issues-unsubscribe@tez.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] TEZ-4757: APPROXIMATE_INPUT_RECORDS over-counts the rows an input will deliver [tez]
Posted by "abstractdog (via GitHub)" <gi...@apache.org>.
abstractdog merged PR #540:
URL: https://github.com/apache/tez/pull/540
--
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: issues-unsubscribe@tez.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] TEZ-4757: APPROXIMATE_INPUT_RECORDS over-counts the rows an input will deliver [tez]
Posted by "tez-yetus (via GitHub)" <gi...@apache.org>.
tez-yetus commented on PR #540:
URL: https://github.com/apache/tez/pull/540#issuecomment-5762330045
:broken_heart: **-1 overall**
| Vote | Subsystem | Runtime | Logfile | Comment |
|:----:|----------:|--------:|:--------:|:-------:|
| +0 :ok: | reexec | 0m 8s | | Docker mode activated. |
|||| _ Prechecks _ |
| +1 :green_heart: | dupname | 0m 0s | | No case conflicting files found. |
| +0 :ok: | detsecrets | 0m 0s | | detect-secrets was not available. |
| +1 :green_heart: | @author | 0m 0s | | The patch does not contain any @author tags. |
| +1 :green_heart: | test4tests | 0m 0s | | The patch appears to include 3 new or modified test files. |
|||| _ master Compile Tests _ |
| +1 :green_heart: | mvninstall | 3m 2s | | master passed |
| +1 :green_heart: | compile | 2m 34s | | master passed |
| -1 :x: | bufcompat | 0m 1s | [/branch-bufcompat-stderr.txt](https://ci-hadoop.apache.org/job/tez-multibranch/job/PR-540/4/artifact/out/branch-bufcompat-stderr.txt) | Error running buf. Please check buf stderr files. |
| +1 :green_heart: | checkstyle | 0m 26s | | master passed |
| +1 :green_heart: | javadoc | 0m 24s | | master passed |
| +0 :ok: | spotbugs | 0m 40s | | tez-runtime-library in master has 143 extant spotbugs warnings. |
|||| _ Patch Compile Tests _ |
| +1 :green_heart: | mvninstall | 2m 14s | | the patch passed |
| -1 :x: | codespell | 0m 52s | [/results-codespell.txt](https://ci-hadoop.apache.org/job/tez-multibranch/job/PR-540/4/artifact/out/results-codespell.txt) | The patch generated 1 new + 10 unchanged - 0 fixed = 11 total (was 10) |
| +1 :green_heart: | compile | 2m 30s | | the patch passed |
| +1 :green_heart: | cc | 2m 30s | | the patch passed |
| -1 :x: | javac | 2m 30s | [/results-compile-javac-root.txt](https://ci-hadoop.apache.org/job/tez-multibranch/job/PR-540/4/artifact/out/results-compile-javac-root.txt) | root generated 2 new + 706 unchanged - 0 fixed = 708 total (was 706) |
| +1 :green_heart: | blanks | 0m 0s | | The patch has no blanks issues. |
| +1 :green_heart: | buflint | 0m 5s | | No new issues. |
| -0 :warning: | checkstyle | 0m 18s | [/results-checkstyle-tez-runtime-library.txt](https://ci-hadoop.apache.org/job/tez-multibranch/job/PR-540/4/artifact/out/results-checkstyle-tez-runtime-library.txt) | tez-runtime-library: The patch generated 3 new + 183 unchanged - 1 fixed = 186 total (was 184) |
| +1 :green_heart: | javadoc | 0m 19s | | the patch passed |
| +1 :green_heart: | spotbugs | 0m 47s | | the patch passed |
|||| _ Other Tests _ |
| +1 :green_heart: | unit | 59m 41s | | root in the patch passed. |
| +1 :green_heart: | asflicense | 0m 23s | | The patch does not generate ASF License warnings. |
| | | 75m 24s | | |
| Subsystem | Report/Notes |
|----------:|:-------------|
| Docker | ClientAPI=1.56 ServerAPI=1.56 base: https://ci-hadoop.apache.org/job/tez-multibranch/job/PR-540/4/artifact/out/Dockerfile |
| Optional Tests | dupname compile unit asflicense javac javadoc spotbugs checkstyle codespell detsecrets cc buflint bufcompat |
| uname | Linux 7793c0caf8ad 5.15.0-190-generic #200-Ubuntu SMP Fri Aug 7 15:06:04 UTC 2026 x86_64 x86_64 x86_64 GNU/Linux |
| Build tool | maven |
| Personality | tez-personality.sh |
| git revision | master / 0e9f756609fe06835cb8f50e0f8e90a3e8282a6f |
| Default Java | Eclipse Adoptium-21.0.12+8-LTS |
| Test Results | https://ci-hadoop.apache.org/job/tez-multibranch/job/PR-540/4/testReport/ |
| Max. process+thread count | 1417 (vs. ulimit of 5500) |
| modules | C: tez-runtime-library U: tez-runtime-library |
| Console output | https://ci-hadoop.apache.org/job/tez-multibranch/job/PR-540/4/console |
| versions | git=2.43.0 maven=3.9.15 buf=1.68.2 spotbugs=4.9.3 codespell=2.4.1 |
| Powered by | Apache Yetus 0.15.1 https://yetus.apache.org |
This message was automatically generated.
--
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: issues-unsubscribe@tez.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] TEZ-4757: APPROXIMATE_INPUT_RECORDS over-counts the rows an input will deliver [tez]
Posted by "tez-yetus (via GitHub)" <gi...@apache.org>.
tez-yetus commented on PR #540:
URL: https://github.com/apache/tez/pull/540#issuecomment-5742287577
:confetti_ball: **+1 overall**
| Vote | Subsystem | Runtime | Logfile | Comment |
|:----:|----------:|--------:|:--------:|:-------:|
| +0 :ok: | reexec | 0m 12s | | Docker mode activated. |
|||| _ Prechecks _ |
| +1 :green_heart: | dupname | 0m 0s | | No case conflicting files found. |
| +0 :ok: | detsecrets | 0m 0s | | detect-secrets was not available. |
| +1 :green_heart: | @author | 0m 0s | | The patch does not contain any @author tags. |
| +1 :green_heart: | test4tests | 0m 0s | | The patch appears to include 2 new or modified test files. |
|||| _ master Compile Tests _ |
| +1 :green_heart: | mvninstall | 4m 50s | | master passed |
| +1 :green_heart: | compile | 4m 0s | | master passed |
| +1 :green_heart: | checkstyle | 0m 36s | | master passed |
| +1 :green_heart: | javadoc | 0m 38s | | master passed |
| +0 :ok: | spotbugs | 1m 14s | | tez-runtime-library in master has 143 extant spotbugs warnings. |
|||| _ Patch Compile Tests _ |
| +1 :green_heart: | mvninstall | 3m 40s | | the patch passed |
| +1 :green_heart: | codespell | 1m 34s | | No new issues. |
| +1 :green_heart: | compile | 4m 4s | | the patch passed |
| +1 :green_heart: | javac | 4m 4s | | the patch passed |
| +1 :green_heart: | blanks | 0m 0s | | The patch has no blanks issues. |
| -0 :warning: | checkstyle | 0m 33s | [/results-checkstyle-tez-runtime-library.txt](https://ci-hadoop.apache.org/job/tez-multibranch/job/PR-540/1/artifact/out/results-checkstyle-tez-runtime-library.txt) | tez-runtime-library: The patch generated 1 new + 77 unchanged - 1 fixed = 78 total (was 78) |
| +1 :green_heart: | javadoc | 0m 38s | | the patch passed |
| +1 :green_heart: | spotbugs | 1m 28s | | the patch passed |
|||| _ Other Tests _ |
| +1 :green_heart: | unit | 74m 0s | | root in the patch passed. |
| +1 :green_heart: | asflicense | 0m 32s | | The patch does not generate ASF License warnings. |
| | | 99m 39s | | |
| Subsystem | Report/Notes |
|----------:|:-------------|
| Docker | ClientAPI=1.56 ServerAPI=1.56 base: https://ci-hadoop.apache.org/job/tez-multibranch/job/PR-540/1/artifact/out/Dockerfile |
| Optional Tests | dupname compile unit asflicense javac javadoc spotbugs checkstyle codespell detsecrets |
| uname | Linux c0c85768ffd3 5.15.0-190-generic #200-Ubuntu SMP Fri Aug 7 15:06:04 UTC 2026 x86_64 x86_64 x86_64 GNU/Linux |
| Build tool | maven |
| Personality | tez-personality.sh |
| git revision | master / 0e9f756609fe06835cb8f50e0f8e90a3e8282a6f |
| Default Java | Eclipse Adoptium-21.0.12+8-LTS |
| Test Results | https://ci-hadoop.apache.org/job/tez-multibranch/job/PR-540/1/testReport/ |
| Max. process+thread count | 1405 (vs. ulimit of 5500) |
| modules | C: tez-runtime-library U: tez-runtime-library |
| Console output | https://ci-hadoop.apache.org/job/tez-multibranch/job/PR-540/1/console |
| versions | git=2.43.0 maven=3.9.15 spotbugs=4.9.3 codespell=2.4.1 |
| Powered by | Apache Yetus 0.15.1 https://yetus.apache.org |
This message was automatically generated.
--
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: issues-unsubscribe@tez.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] TEZ-4757: APPROXIMATE_INPUT_RECORDS over-counts the rows an input will deliver [tez]
Posted by "deniskuzZ (via GitHub)" <gi...@apache.org>.
deniskuzZ commented on PR #540:
URL: https://github.com/apache/tez/pull/540#issuecomment-5915691457
thank for the review @abstractdog! I've addressed the comments, please take a look once you get a chance
--
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: issues-unsubscribe@tez.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] TEZ-4757: APPROXIMATE_INPUT_RECORDS over-counts the rows an input will deliver [tez]
Posted by "abstractdog (via GitHub)" <gi...@apache.org>.
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: issues-unsubscribe@tez.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] TEZ-4757: APPROXIMATE_INPUT_RECORDS over-counts the rows an input will deliver [tez]
Posted by "tez-yetus (via GitHub)" <gi...@apache.org>.
tez-yetus commented on PR #540:
URL: https://github.com/apache/tez/pull/540#issuecomment-5765058295
:broken_heart: **-1 overall**
| Vote | Subsystem | Runtime | Logfile | Comment |
|:----:|----------:|--------:|:--------:|:-------:|
| +0 :ok: | reexec | 0m 12s | | Docker mode activated. |
|||| _ Prechecks _ |
| +1 :green_heart: | dupname | 0m 0s | | No case conflicting files found. |
| +0 :ok: | detsecrets | 0m 0s | | detect-secrets was not available. |
| +1 :green_heart: | @author | 0m 0s | | The patch does not contain any @author tags. |
| +1 :green_heart: | test4tests | 0m 0s | | The patch appears to include 4 new or modified test files. |
|||| _ master Compile Tests _ |
| +1 :green_heart: | mvninstall | 5m 4s | | master passed |
| +1 :green_heart: | compile | 4m 4s | | master passed |
| -1 :x: | bufcompat | 0m 2s | [/branch-bufcompat-stderr.txt](https://ci-hadoop.apache.org/job/tez-multibranch/job/PR-540/8/artifact/out/branch-bufcompat-stderr.txt) | Error running buf. Please check buf stderr files. |
| +1 :green_heart: | checkstyle | 0m 41s | | master passed |
| +1 :green_heart: | javadoc | 1m 2s | | master passed |
| +0 :ok: | spotbugs | 1m 18s | | tez-runtime-library in master has 143 extant spotbugs warnings. |
|||| _ Patch Compile Tests _ |
| +1 :green_heart: | mvninstall | 3m 43s | | the patch passed |
| -1 :x: | codespell | 1m 34s | [/results-codespell.txt](https://ci-hadoop.apache.org/job/tez-multibranch/job/PR-540/8/artifact/out/results-codespell.txt) | The patch generated 1 new + 13 unchanged - 0 fixed = 14 total (was 13) |
| +1 :green_heart: | compile | 4m 5s | | the patch passed |
| +1 :green_heart: | cc | 4m 5s | | the patch passed |
| -1 :x: | javac | 4m 5s | [/results-compile-javac-root.txt](https://ci-hadoop.apache.org/job/tez-multibranch/job/PR-540/8/artifact/out/results-compile-javac-root.txt) | root generated 2 new + 706 unchanged - 0 fixed = 708 total (was 706) |
| +1 :green_heart: | blanks | 0m 0s | | The patch has no blanks issues. |
| +1 :green_heart: | buflint | 0m 9s | | No new issues. |
| -0 :warning: | checkstyle | 0m 34s | [/results-checkstyle-tez-runtime-library.txt](https://ci-hadoop.apache.org/job/tez-multibranch/job/PR-540/8/artifact/out/results-checkstyle-tez-runtime-library.txt) | tez-runtime-library: The patch generated 6 new + 217 unchanged - 1 fixed = 223 total (was 218) |
| +1 :green_heart: | javadoc | 0m 38s | | the patch passed |
| +1 :green_heart: | spotbugs | 1m 26s | | the patch passed |
|||| _ Other Tests _ |
| +1 :green_heart: | unit | 75m 42s | | root in the patch passed. |
| +1 :green_heart: | asflicense | 0m 33s | | The patch does not generate ASF License warnings. |
| | | 102m 24s | | |
| Subsystem | Report/Notes |
|----------:|:-------------|
| Docker | ClientAPI=1.56 ServerAPI=1.56 base: https://ci-hadoop.apache.org/job/tez-multibranch/job/PR-540/8/artifact/out/Dockerfile |
| Optional Tests | dupname compile unit asflicense javac javadoc spotbugs checkstyle codespell detsecrets cc buflint bufcompat |
| uname | Linux 9235b2c4fc18 5.15.0-190-generic #200-Ubuntu SMP Fri Aug 7 15:06:04 UTC 2026 x86_64 x86_64 x86_64 GNU/Linux |
| Build tool | maven |
| Personality | tez-personality.sh |
| git revision | master / 0e9f756609fe06835cb8f50e0f8e90a3e8282a6f |
| Default Java | Eclipse Adoptium-21.0.12+8-LTS |
| Test Results | https://ci-hadoop.apache.org/job/tez-multibranch/job/PR-540/8/testReport/ |
| Max. process+thread count | 2124 (vs. ulimit of 5500) |
| modules | C: tez-runtime-library U: tez-runtime-library |
| Console output | https://ci-hadoop.apache.org/job/tez-multibranch/job/PR-540/8/console |
| versions | git=2.43.0 maven=3.9.15 buf=1.68.2 spotbugs=4.9.3 codespell=2.4.1 |
| Powered by | Apache Yetus 0.15.1 https://yetus.apache.org |
This message was automatically generated.
--
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: issues-unsubscribe@tez.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] TEZ-4757: APPROXIMATE_INPUT_RECORDS over-counts the rows an input will deliver [tez]
Posted by "deniskuzZ (via GitHub)" <gi...@apache.org>.
deniskuzZ commented on code in PR #540:
URL: https://github.com/apache/tez/pull/540#discussion_r4144780876
##########
tez-runtime-library/src/test/java/org/apache/tez/runtime/library/common/writers/TestUnorderedPartitionedKVWriter.java:
##########
@@ -432,6 +433,72 @@ public void testTextMixedRecordsWithoutFinalMerge(boolean shouldCompress, Report
textTest(100, 10, 2048, 10, 10, 10, false, false);
}
+ /**
+ * One partition with pipelined shuffle is the only shape where writeLargeRecord runs while
+ * num_record is set, so it is the only one that can show the DME under-counting large records.
+ */
+ @ParameterizedTest(name = "test[{0}, {1}]")
+ @MethodSource("data")
+ @Timeout(value = 10000, unit = TimeUnit.MILLISECONDS)
+ public void testLargeRecords_SinglePartition(boolean shouldCompress,
+ ReportPartitionStats reportPartitionStats) throws IOException, InterruptedException {
+ setupInit(shouldCompress, reportPartitionStats);
+ textTest(0, 1, 2048, 0, 0, 5, true, false);
+ }
+
+ /**
+ * An output small enough to fit one buffer spills only at close, so its records reach the task
+ * counters in the same call that builds the final event -- which is where a count read too
+ * early reports zero.
+ */
+ @ParameterizedTest(name = "test[{0}, {1}]")
+ @MethodSource("data")
+ @Timeout(value = 10000, unit = TimeUnit.MILLISECONDS)
+ public void testFinalEventRecordCount_SinglePartition(boolean shouldCompress,
+ ReportPartitionStats reportPartitionStats) throws IOException, InterruptedException {
+ setupInit(shouldCompress, reportPartitionStats);
+ textTest(5, 1, 2048, 0, 0, 0, true, false);
+ }
+
+ /**
+ * A single-partition writer is the only one that fills num_record, and large records never
+ * reach outputRecordsCounter -- writeLargeRecord bypasses it -- so both the DME and the
+ * VertexManager event have to carry the sum of the two counters. Under pipelined shuffle the
+ * events are pushed through sendEvents rather than returned by close(), so both sources are
+ * scanned and the last of each kind is the one that has to be right.
+ */
+ private void assertLastEventsCountEveryRecord(UnorderedPartitionedKVWriter kvWriter,
+ OutputContext outputContext, List<Event> closeEvents) throws IOException {
+ List<Event> allEvents = new ArrayList<>(closeEvents);
+ @SuppressWarnings("unchecked")
+ ArgumentCaptor<List<Event>> sent = ArgumentCaptor.forClass(List.class);
+ verify(outputContext, atLeast(0)).sendEvents(sent.capture());
+ sent.getAllValues().forEach(allEvents::addAll);
+
+ long expected = kvWriter.outputRecordsCounter.getValue()
+ + kvWriter.outputLargeRecordsCounter.getValue();
+ long lastDmeCount = -1;
+ long lastVmCount = -1;
+ for (Event event : allEvents) {
+ if (event instanceof CompositeDataMovementEvent) {
+ ShuffleUserPayloads.DataMovementEventPayloadProto dme =
+ ShuffleUserPayloads.DataMovementEventPayloadProto.parseFrom(ByteString.copyFrom(
+ ((CompositeDataMovementEvent) event).getUserPayload()));
+ if (dme.hasNumRecord()) {
+ lastDmeCount = dme.getNumRecord();
+ }
+ } else if (event instanceof VertexManagerEvent) {
+ ShuffleUserPayloads.VertexManagerEventPayloadProto vme =
+ ShuffleUserPayloads.VertexManagerEventPayloadProto.parseFrom(ByteString.copyFrom(
+ ((VertexManagerEvent) event).getUserPayload()));
+ lastVmCount = vme.getNumRecord();
+ }
+ }
+ assertTrue(lastDmeCount >= 0, "expected a DME carrying num_record");
+ assertEquals(expected, lastDmeCount, "DME");
+ assertEquals(expected, lastVmCount, "VertexManagerEvent");
Review Comment:
fixed
--
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: issues-unsubscribe@tez.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] TEZ-4757: APPROXIMATE_INPUT_RECORDS over-counts the rows an input will deliver [tez]
Posted by "deniskuzZ (via GitHub)" <gi...@apache.org>.
deniskuzZ commented on code in PR #540:
URL: https://github.com/apache/tez/pull/540#discussion_r4144654893
##########
tez-runtime-library/src/test/java/org/apache/tez/runtime/library/common/shuffle/impl/TestShuffleInputEventHandlerImpl.java:
##########
@@ -408,6 +408,63 @@ public void testDataMovementEventsWithShuffleData() throws IOException {
}
}
+ /**
+ * An input whose partition is empty reports zero rows and must still reach the ShuffleManager:
+ * counting it only in numInputs and not in the denominator is what inflated the counter.
+ */
+ @Test
+ @Timeout(value = 5000, unit = TimeUnit.MILLISECONDS)
+ public void testAnEmptyPartitionStillReportsItsRecordCount() throws IOException {
+ ShuffleManager shuffleManager = mock(ShuffleManager.class);
+ ShuffleInputEventHandlerImpl handler = newHandler(shuffleManager, false);
+
+ handler.handleEvents(Collections.singletonList(
+ createRecordCountEvent(1, 0, createEmptyPartitionByteString(0), false)));
+
+ verify(shuffleManager).updateApproximateInputRecords(eq(1), eq(0L));
+ verify(shuffleManager).addCompletedInputWithNoData(any());
+ }
+
+ /** The composite path carries the same payload and has to report it too. */
+ @Test
+ @Timeout(value = 5000, unit = TimeUnit.MILLISECONDS)
+ public void testACompositeEventReportsItsRecordCount() throws IOException {
+ ShuffleManager shuffleManager = mock(ShuffleManager.class);
+ ShuffleInputEventHandlerImpl handler = newHandler(shuffleManager, true);
+
+ handler.handleEvents(Collections.singletonList(createRecordCountEvent(1, 4000, null, true)));
+
+ verify(shuffleManager).updateApproximateInputRecords(eq(1), eq(4000L));
+ }
+
+ /**
+ * A source task can emit more rows than an int32 holds. num_record is an int64 so the count
+ * survives the wire; passing it as an int would not even compile against that field.
+ */
+ @Test
+ @Timeout(value = 5000, unit = TimeUnit.MILLISECONDS)
+ public void testACountAboveIntMaxSurvivesTheWire() throws IOException {
+ ShuffleManager shuffleManager = mock(ShuffleManager.class);
+ ShuffleInputEventHandlerImpl handler = newHandler(shuffleManager, false);
+
+ handler.handleEvents(Collections.singletonList(
+ createRecordCountEvent(1, 3_000_000_000L, null, false)));
+
+ verify(shuffleManager).updateApproximateInputRecords(eq(1), eq(3_000_000_000L));
+ }
+
+ /** A payload with no record count -- any multi-partition writer -- reports nothing. */
+ @Test
+ @Timeout(value = 5000, unit = TimeUnit.MILLISECONDS)
+ public void testAPayloadWithoutARecordCountReportsNothing() throws IOException {
+ ShuffleManager shuffleManager = mock(ShuffleManager.class);
+ ShuffleInputEventHandlerImpl handler = newHandler(shuffleManager, false);
+
+ handler.handleEvents(Collections.singletonList(createDataMovementEvent(0, 1, null)));
+
+ verify(shuffleManager, times(0)).updateApproximateInputRecords(anyInt(), anyLong());
+ }
Review Comment:
fixed
--
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: issues-unsubscribe@tez.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] TEZ-4757: APPROXIMATE_INPUT_RECORDS over-counts the rows an input will deliver [tez]
Posted by "deniskuzZ (via GitHub)" <gi...@apache.org>.
deniskuzZ commented on code in PR #540:
URL: https://github.com/apache/tez/pull/540#discussion_r4144764948
##########
tez-runtime-library/src/test/java/org/apache/tez/runtime/library/common/writers/TestUnorderedPartitionedKVWriter.java:
##########
@@ -432,6 +433,72 @@ public void testTextMixedRecordsWithoutFinalMerge(boolean shouldCompress, Report
textTest(100, 10, 2048, 10, 10, 10, false, false);
}
+ /**
Review Comment:
testNoRecords_SinglePartition_WithPipelinedShuffle already runs exactly that shape through baseTestWithPipelinedTransfer
--
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: issues-unsubscribe@tez.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] TEZ-4757: APPROXIMATE_INPUT_RECORDS over-counts the rows an input will deliver [tez]
Posted by "tez-yetus (via GitHub)" <gi...@apache.org>.
tez-yetus commented on PR #540:
URL: https://github.com/apache/tez/pull/540#issuecomment-5785663942
:broken_heart: **-1 overall**
| Vote | Subsystem | Runtime | Logfile | Comment |
|:----:|----------:|--------:|:--------:|:-------:|
| +0 :ok: | reexec | 0m 13s | | Docker mode activated. |
|||| _ Prechecks _ |
| +1 :green_heart: | dupname | 0m 0s | | No case conflicting files found. |
| +0 :ok: | detsecrets | 0m 0s | | detect-secrets was not available. |
| +1 :green_heart: | @author | 0m 0s | | The patch does not contain any @author tags. |
| +1 :green_heart: | test4tests | 0m 0s | | The patch appears to include 4 new or modified test files. |
|||| _ master Compile Tests _ |
| +0 :ok: | mvndep | 0m 50s | | Maven dependency ordering for branch |
| +1 :green_heart: | mvninstall | 4m 39s | | master passed |
| +1 :green_heart: | compile | 4m 1s | | master passed |
| -1 :x: | bufcompat | 0m 2s | [/branch-bufcompat-stderr.txt](https://ci-hadoop.apache.org/job/tez-multibranch/job/PR-540/9/artifact/out/branch-bufcompat-stderr.txt) | Error running buf. Please check buf stderr files. |
| +1 :green_heart: | checkstyle | 1m 39s | | master passed |
| +1 :green_heart: | javadoc | 2m 9s | | master passed |
| +0 :ok: | spotbugs | 1m 15s | | tez-runtime-library in master has 143 extant spotbugs warnings. |
| +0 :ok: | spotbugs | 6m 37s | | root in master has 1054 extant spotbugs warnings. |
|||| _ Patch Compile Tests _ |
| +0 :ok: | mvndep | 0m 9s | | Maven dependency ordering for patch |
| +1 :green_heart: | mvninstall | 3m 53s | | the patch passed |
| +1 :green_heart: | codespell | 1m 34s | | The patch generated 0 new + 5 unchanged - 8 fixed = 5 total (was 13) |
| +1 :green_heart: | compile | 4m 2s | | the patch passed |
| +1 :green_heart: | cc | 4m 2s | | the patch passed |
| +1 :green_heart: | javac | 4m 2s | | the patch passed |
| +1 :green_heart: | blanks | 0m 0s | | The patch has no blanks issues. |
| +1 :green_heart: | buflint | 0m 9s | | No new issues. |
| -0 :warning: | checkstyle | 0m 32s | [/results-checkstyle-tez-runtime-library.txt](https://ci-hadoop.apache.org/job/tez-multibranch/job/PR-540/9/artifact/out/results-checkstyle-tez-runtime-library.txt) | tez-runtime-library: The patch generated 6 new + 217 unchanged - 1 fixed = 223 total (was 218) |
| -0 :warning: | checkstyle | 1m 0s | [/results-checkstyle-root.txt](https://ci-hadoop.apache.org/job/tez-multibranch/job/PR-540/9/artifact/out/results-checkstyle-root.txt) | root: The patch generated 6 new + 217 unchanged - 1 fixed = 223 total (was 218) |
| +1 :green_heart: | javadoc | 2m 4s | | the patch passed |
| +1 :green_heart: | spotbugs | 8m 9s | | the patch passed |
|||| _ Other Tests _ |
| +1 :green_heart: | unit | 75m 39s | | root in the patch passed. |
| +1 :green_heart: | asflicense | 1m 22s | | The patch does not generate ASF License warnings. |
| | | 122m 50s | | |
| Subsystem | Report/Notes |
|----------:|:-------------|
| Docker | ClientAPI=1.56 ServerAPI=1.56 base: https://ci-hadoop.apache.org/job/tez-multibranch/job/PR-540/9/artifact/out/Dockerfile |
| Optional Tests | dupname compile unit asflicense codespell detsecrets javac javadoc spotbugs checkstyle cc buflint bufcompat |
| uname | Linux c22f869cb033 5.15.0-190-generic #200-Ubuntu SMP Fri Aug 7 15:06:04 UTC 2026 x86_64 x86_64 x86_64 GNU/Linux |
| Build tool | maven |
| Personality | tez-personality.sh |
| git revision | master / 0e9f756609fe06835cb8f50e0f8e90a3e8282a6f |
| Default Java | Eclipse Adoptium-21.0.12+8-LTS |
| Test Results | https://ci-hadoop.apache.org/job/tez-multibranch/job/PR-540/9/testReport/ |
| Max. process+thread count | 1372 (vs. ulimit of 5500) |
| modules | C: tez-runtime-library . U: . |
| Console output | https://ci-hadoop.apache.org/job/tez-multibranch/job/PR-540/9/console |
| versions | git=2.43.0 maven=3.9.15 buf=1.68.2 spotbugs=4.9.3 codespell=2.4.1 |
| Powered by | Apache Yetus 0.15.1 https://yetus.apache.org |
This message was automatically generated.
--
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: issues-unsubscribe@tez.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] TEZ-4757: APPROXIMATE_INPUT_RECORDS over-counts the rows an input will deliver [tez]
Posted by "tez-yetus (via GitHub)" <gi...@apache.org>.
tez-yetus commented on PR #540:
URL: https://github.com/apache/tez/pull/540#issuecomment-5817663564
:broken_heart: **-1 overall**
| Vote | Subsystem | Runtime | Logfile | Comment |
|:----:|----------:|--------:|:--------:|:-------:|
| +0 :ok: | reexec | 0m 15s | | Docker mode activated. |
|||| _ Prechecks _ |
| +1 :green_heart: | dupname | 0m 0s | | No case conflicting files found. |
| +0 :ok: | detsecrets | 0m 0s | | detect-secrets was not available. |
| +1 :green_heart: | @author | 0m 0s | | The patch does not contain any @author tags. |
| +1 :green_heart: | test4tests | 0m 0s | | The patch appears to include 4 new or modified test files. |
|||| _ master Compile Tests _ |
| +0 :ok: | mvndep | 0m 53s | | Maven dependency ordering for branch |
| +1 :green_heart: | mvninstall | 5m 5s | | master passed |
| +1 :green_heart: | compile | 4m 4s | | master passed |
| -1 :x: | bufcompat | 0m 2s | [/branch-bufcompat-stderr.txt](https://ci-hadoop.apache.org/job/tez-multibranch/job/PR-540/10/artifact/out/branch-bufcompat-stderr.txt) | Error running buf. Please check buf stderr files. |
| +1 :green_heart: | checkstyle | 1m 46s | | master passed |
| +1 :green_heart: | javadoc | 2m 8s | | master passed |
| +0 :ok: | spotbugs | 1m 18s | | tez-runtime-library in master has 143 extant spotbugs warnings. |
| +0 :ok: | spotbugs | 6m 32s | | root in master has 1054 extant spotbugs warnings. |
|||| _ Patch Compile Tests _ |
| +0 :ok: | mvndep | 0m 9s | | Maven dependency ordering for patch |
| +1 :green_heart: | mvninstall | 4m 25s | | the patch passed |
| +1 :green_heart: | codespell | 1m 52s | | The patch generated 0 new + 5 unchanged - 8 fixed = 5 total (was 13) |
| +1 :green_heart: | compile | 4m 15s | | the patch passed |
| +1 :green_heart: | cc | 4m 15s | | the patch passed |
| +1 :green_heart: | javac | 4m 15s | | the patch passed |
| +1 :green_heart: | blanks | 0m 0s | | The patch has no blanks issues. |
| +1 :green_heart: | buflint | 0m 9s | | No new issues. |
| -0 :warning: | checkstyle | 0m 30s | [/results-checkstyle-tez-runtime-library.txt](https://ci-hadoop.apache.org/job/tez-multibranch/job/PR-540/10/artifact/out/results-checkstyle-tez-runtime-library.txt) | tez-runtime-library: The patch generated 6 new + 217 unchanged - 1 fixed = 223 total (was 218) |
| -0 :warning: | checkstyle | 1m 2s | [/results-checkstyle-root.txt](https://ci-hadoop.apache.org/job/tez-multibranch/job/PR-540/10/artifact/out/results-checkstyle-root.txt) | root: The patch generated 6 new + 217 unchanged - 1 fixed = 223 total (was 218) |
| +1 :green_heart: | javadoc | 2m 7s | | the patch passed |
| +1 :green_heart: | spotbugs | 8m 34s | | the patch passed |
|||| _ Other Tests _ |
| +1 :green_heart: | unit | 75m 19s | | root in the patch passed. |
| +1 :green_heart: | asflicense | 1m 24s | | The patch does not generate ASF License warnings. |
| | | 124m 51s | | |
| Subsystem | Report/Notes |
|----------:|:-------------|
| Docker | ClientAPI=1.56 ServerAPI=1.56 base: https://ci-hadoop.apache.org/job/tez-multibranch/job/PR-540/10/artifact/out/Dockerfile |
| Optional Tests | dupname compile unit asflicense codespell detsecrets javac javadoc spotbugs checkstyle cc buflint bufcompat |
| uname | Linux 5d6e8734283d 5.15.0-190-generic #200-Ubuntu SMP Fri Aug 7 15:06:04 UTC 2026 x86_64 x86_64 x86_64 GNU/Linux |
| Build tool | maven |
| Personality | tez-personality.sh |
| git revision | master / 0e9f756609fe06835cb8f50e0f8e90a3e8282a6f |
| Default Java | Eclipse Adoptium-21.0.12+8-LTS |
| Test Results | https://ci-hadoop.apache.org/job/tez-multibranch/job/PR-540/10/testReport/ |
| Max. process+thread count | 1544 (vs. ulimit of 5500) |
| modules | C: tez-runtime-library . U: . |
| Console output | https://ci-hadoop.apache.org/job/tez-multibranch/job/PR-540/10/console |
| versions | git=2.43.0 maven=3.9.15 buf=1.68.2 spotbugs=4.9.3 codespell=2.4.1 |
| Powered by | Apache Yetus 0.15.1 https://yetus.apache.org |
This message was automatically generated.
--
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: issues-unsubscribe@tez.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] TEZ-4757: APPROXIMATE_INPUT_RECORDS over-counts the rows an input will deliver [tez]
Posted by "deniskuzZ (via GitHub)" <gi...@apache.org>.
deniskuzZ commented on code in PR #540:
URL: https://github.com/apache/tez/pull/540#discussion_r4144773686
##########
tez-runtime-library/src/test/java/org/apache/tez/runtime/library/common/writers/TestUnorderedPartitionedKVWriter.java:
##########
@@ -432,6 +433,72 @@ public void testTextMixedRecordsWithoutFinalMerge(boolean shouldCompress, Report
textTest(100, 10, 2048, 10, 10, 10, false, false);
}
+ /**
+ * One partition with pipelined shuffle is the only shape where writeLargeRecord runs while
+ * num_record is set, so it is the only one that can show the DME under-counting large records.
+ */
+ @ParameterizedTest(name = "test[{0}, {1}]")
+ @MethodSource("data")
+ @Timeout(value = 10000, unit = TimeUnit.MILLISECONDS)
+ public void testLargeRecords_SinglePartition(boolean shouldCompress,
+ ReportPartitionStats reportPartitionStats) throws IOException, InterruptedException {
+ setupInit(shouldCompress, reportPartitionStats);
+ textTest(0, 1, 2048, 0, 0, 5, true, false);
+ }
+
+ /**
+ * An output small enough to fit one buffer spills only at close, so its records reach the task
+ * counters in the same call that builds the final event -- which is where a count read too
+ * early reports zero.
+ */
+ @ParameterizedTest(name = "test[{0}, {1}]")
+ @MethodSource("data")
+ @Timeout(value = 10000, unit = TimeUnit.MILLISECONDS)
+ public void testFinalEventRecordCount_SinglePartition(boolean shouldCompress,
+ ReportPartitionStats reportPartitionStats) throws IOException, InterruptedException {
+ setupInit(shouldCompress, reportPartitionStats);
+ textTest(5, 1, 2048, 0, 0, 0, true, false);
+ }
+
+ /**
+ * A single-partition writer is the only one that fills num_record, and large records never
+ * reach outputRecordsCounter -- writeLargeRecord bypasses it -- so both the DME and the
+ * VertexManager event have to carry the sum of the two counters. Under pipelined shuffle the
+ * events are pushed through sendEvents rather than returned by close(), so both sources are
+ * scanned and the last of each kind is the one that has to be right.
+ */
+ private void assertLastEventsCountEveryRecord(UnorderedPartitionedKVWriter kvWriter,
+ OutputContext outputContext, List<Event> closeEvents) throws IOException {
+ List<Event> allEvents = new ArrayList<>(closeEvents);
+ @SuppressWarnings("unchecked")
+ ArgumentCaptor<List<Event>> sent = ArgumentCaptor.forClass(List.class);
+ verify(outputContext, atLeast(0)).sendEvents(sent.capture());
+ sent.getAllValues().forEach(allEvents::addAll);
+
+ long expected = kvWriter.outputRecordsCounter.getValue()
+ + kvWriter.outputLargeRecordsCounter.getValue();
Review Comment:
changed
--
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: issues-unsubscribe@tez.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] TEZ-4757: APPROXIMATE_INPUT_RECORDS over-counts the rows an input will deliver [tez]
Posted by "deniskuzZ (via GitHub)" <gi...@apache.org>.
deniskuzZ commented on code in PR #540:
URL: https://github.com/apache/tez/pull/540#discussion_r4144657747
##########
tez-runtime-library/src/test/java/org/apache/tez/runtime/library/common/writers/TestUnorderedPartitionedKVWriter.java:
##########
@@ -432,6 +433,72 @@ public void testTextMixedRecordsWithoutFinalMerge(boolean shouldCompress, Report
textTest(100, 10, 2048, 10, 10, 10, false, false);
}
+ /**
+ * One partition with pipelined shuffle is the only shape where writeLargeRecord runs while
+ * num_record is set, so it is the only one that can show the DME under-counting large records.
+ */
+ @ParameterizedTest(name = "test[{0}, {1}]")
+ @MethodSource("data")
+ @Timeout(value = 10000, unit = TimeUnit.MILLISECONDS)
+ public void testLargeRecords_SinglePartition(boolean shouldCompress,
+ ReportPartitionStats reportPartitionStats) throws IOException, InterruptedException {
+ setupInit(shouldCompress, reportPartitionStats);
+ textTest(0, 1, 2048, 0, 0, 5, true, false);
+ }
+
+ /**
+ * An output small enough to fit one buffer spills only at close, so its records reach the task
+ * counters in the same call that builds the final event -- which is where a count read too
+ * early reports zero.
+ */
+ @ParameterizedTest(name = "test[{0}, {1}]")
+ @MethodSource("data")
+ @Timeout(value = 10000, unit = TimeUnit.MILLISECONDS)
+ public void testFinalEventRecordCount_SinglePartition(boolean shouldCompress,
Review Comment:
renamed
--
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: issues-unsubscribe@tez.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] TEZ-4757: APPROXIMATE_INPUT_RECORDS over-counts the rows an input will deliver [tez]
Posted by "tez-yetus (via GitHub)" <gi...@apache.org>.
tez-yetus commented on PR #540:
URL: https://github.com/apache/tez/pull/540#issuecomment-5913338537
:confetti_ball: **+1 overall**
| Vote | Subsystem | Runtime | Logfile | Comment |
|:----:|----------:|--------:|:--------:|:-------:|
| +0 :ok: | reexec | 0m 8s | | Docker mode activated. |
|||| _ Prechecks _ |
| +1 :green_heart: | dupname | 0m 0s | | No case conflicting files found. |
| +0 :ok: | detsecrets | 0m 0s | | detect-secrets was not available. |
| +1 :green_heart: | @author | 0m 0s | | The patch does not contain any @author tags. |
| +1 :green_heart: | test4tests | 0m 0s | | The patch appears to include 4 new or modified test files. |
|||| _ master Compile Tests _ |
| +0 :ok: | mvndep | 0m 32s | | Maven dependency ordering for branch |
| +1 :green_heart: | mvninstall | 2m 47s | | master passed |
| +1 :green_heart: | compile | 2m 20s | | master passed |
| +1 :green_heart: | checkstyle | 0m 57s | | master passed |
| +1 :green_heart: | javadoc | 1m 13s | | master passed |
| +0 :ok: | spotbugs | 0m 40s | | tez-runtime-library in master has 143 extant spotbugs warnings. |
| +0 :ok: | spotbugs | 3m 39s | | root in master has 1054 extant spotbugs warnings. |
|||| _ Patch Compile Tests _ |
| +0 :ok: | mvndep | 0m 7s | | Maven dependency ordering for patch |
| +1 :green_heart: | mvninstall | 2m 10s | | the patch passed |
| +1 :green_heart: | codespell | 0m 52s | | The patch generated 0 new + 5 unchanged - 8 fixed = 5 total (was 13) |
| +1 :green_heart: | compile | 2m 26s | | the patch passed |
| +1 :green_heart: | cc | 2m 26s | | the patch passed |
| +1 :green_heart: | javac | 2m 26s | | the patch passed |
| +1 :green_heart: | blanks | 0m 0s | | The patch has no blanks issues. |
| +1 :green_heart: | buflint | 0m 2s | | No new issues. |
| -0 :warning: | checkstyle | 0m 17s | [/results-checkstyle-tez-runtime-library.txt](https://ci-hadoop.apache.org/job/tez-multibranch/job/PR-540/14/artifact/out/results-checkstyle-tez-runtime-library.txt) | tez-runtime-library: The patch generated 4 new + 217 unchanged - 1 fixed = 221 total (was 218) |
| -0 :warning: | checkstyle | 0m 34s | [/results-checkstyle-root.txt](https://ci-hadoop.apache.org/job/tez-multibranch/job/PR-540/14/artifact/out/results-checkstyle-root.txt) | root: The patch generated 4 new + 217 unchanged - 1 fixed = 221 total (was 218) |
| +1 :green_heart: | javadoc | 1m 9s | | the patch passed |
| +1 :green_heart: | spotbugs | 4m 35s | | the patch passed |
|||| _ Other Tests _ |
| +1 :green_heart: | unit | 56m 39s | | root in the patch passed. |
| +1 :green_heart: | asflicense | 0m 44s | | The patch does not generate ASF License warnings. |
| | | 83m 47s | | |
| Subsystem | Report/Notes |
|----------:|:-------------|
| Docker | ClientAPI=1.56 ServerAPI=1.56 base: https://ci-hadoop.apache.org/job/tez-multibranch/job/PR-540/14/artifact/out/Dockerfile |
| Optional Tests | dupname compile unit asflicense codespell detsecrets javac javadoc spotbugs checkstyle cc buflint bufcompat |
| uname | Linux b3e832cb2e98 5.15.0-190-generic #200-Ubuntu SMP Fri Aug 7 15:06:04 UTC 2026 x86_64 x86_64 x86_64 GNU/Linux |
| Build tool | maven |
| Personality | tez-personality.sh |
| git revision | master / ae2756ed19f8b757fc3d32fcd9f2485702692e9b |
| Default Java | Eclipse Adoptium-21.0.12.1+1-LTS |
| Test Results | https://ci-hadoop.apache.org/job/tez-multibranch/job/PR-540/14/testReport/ |
| Max. process+thread count | 2127 (vs. ulimit of 5500) |
| modules | C: tez-runtime-library . U: . |
| Console output | https://ci-hadoop.apache.org/job/tez-multibranch/job/PR-540/14/console |
| versions | git=2.43.0 maven=3.9.15 buf=1.68.2 spotbugs=4.9.3 codespell=2.4.1 |
| Powered by | Apache Yetus 0.15.1 https://yetus.apache.org |
This message was automatically generated.
--
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: issues-unsubscribe@tez.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] TEZ-4757: APPROXIMATE_INPUT_RECORDS over-counts the rows an input will deliver [tez]
Posted by "Copilot (via GitHub)" <gi...@apache.org>.
Copilot commented on code in PR #540:
URL: https://github.com/apache/tez/pull/540#discussion_r4060344229
##########
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) {
+ if (numRecords < 0 || inputIndex < 0 || inputIndex >= numInputs) {
return;
}
- inputRecordsFromEvents += delta;
- eventsReceived++;
- approximateInputRecords.setValue((inputRecordsFromEvents / eventsReceived) * numInputs);
+ int reportedSoFar = recordsPerInput[inputIndex];
+ if (reportedSoFar == NO_RECORDS_REPORTED) {
+ inputsReportingRecords++;
+ reportedSoFar = 0;
+ }
+ inputRecordsFromEvents += numRecords - reportedSoFar;
+ recordsPerInput[inputIndex] = numRecords;
+ approximateInputRecords.setValue((inputRecordsFromEvents * numInputs) / inputsReportingRecords);
Review Comment:
This multiplication can overflow `long` before the division. Because each announced value is capped at `Integer.MAX_VALUE`, a vertex with more than about 65K reporting inputs can produce an incorrect (even negative) counter although the final extrapolated total still fits in a `long`. Compute the scaled value with overflow-safe arithmetic (for example, `BigInteger`) instead of multiplying the two large factors directly.
##########
tez-runtime-library/src/main/java/org/apache/tez/runtime/library/common/writers/UnorderedPartitionedKVWriter.java:
##########
@@ -906,7 +906,10 @@ private Event generateDMEvent(boolean addSpillDetails, int spillId,
DataMovementEventPayloadProto.Builder payloadBuilder = DataMovementEventPayloadProto
.newBuilder();
if (numPartitions == 1) {
- payloadBuilder.setNumRecord((int) outputRecordsCounter.getValue());
+ // Saturate rather than wrap: the field is an int32 while the counters are longs. Large
+ // records bypass outputRecordsCounter, so add them the way the VertexManager event does.
+ long records = outputRecordsCounter.getValue() + outputLargeRecordsCounter.getValue();
+ payloadBuilder.setNumRecord((int) Math.min(records, Integer.MAX_VALUE));
Review Comment:
The writer tests cover one-partition payloads only for zero/ordinary records; they do not exercise this new branch with a large record or a count above `Integer.MAX_VALUE`. Since both the `OUTPUT_LARGE_RECORDS` addition and saturation are protocol-visible fixes, add a one-partition DME assertion for those cases so a regression can’t silently reintroduce under-counting or int wrapping.
--
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: issues-unsubscribe@tez.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] TEZ-4757: APPROXIMATE_INPUT_RECORDS over-counts the rows an input will deliver [tez]
Posted by "deniskuzZ (via GitHub)" <gi...@apache.org>.
deniskuzZ commented on code in PR #540:
URL: https://github.com/apache/tez/pull/540#discussion_r4144788407
##########
tez-runtime-library/src/test/java/org/apache/tez/runtime/library/common/writers/TestUnorderedPartitionedKVWriter.java:
##########
@@ -432,6 +433,72 @@ public void testTextMixedRecordsWithoutFinalMerge(boolean shouldCompress, Report
textTest(100, 10, 2048, 10, 10, 10, false, false);
}
+ /**
+ * One partition with pipelined shuffle is the only shape where writeLargeRecord runs while
+ * num_record is set, so it is the only one that can show the DME under-counting large records.
+ */
+ @ParameterizedTest(name = "test[{0}, {1}]")
+ @MethodSource("data")
+ @Timeout(value = 10000, unit = TimeUnit.MILLISECONDS)
+ public void testLargeRecords_SinglePartition(boolean shouldCompress,
+ ReportPartitionStats reportPartitionStats) throws IOException, InterruptedException {
+ setupInit(shouldCompress, reportPartitionStats);
+ textTest(0, 1, 2048, 0, 0, 5, true, false);
+ }
+
+ /**
+ * An output small enough to fit one buffer spills only at close, so its records reach the task
+ * counters in the same call that builds the final event -- which is where a count read too
+ * early reports zero.
Review Comment:
changed
--
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: issues-unsubscribe@tez.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] TEZ-4757: APPROXIMATE_INPUT_RECORDS over-counts the rows an input will deliver [tez]
Posted by "deniskuzZ (via GitHub)" <gi...@apache.org>.
deniskuzZ commented on code in PR #540:
URL: https://github.com/apache/tez/pull/540#discussion_r4144838032
##########
tez-runtime-library/src/test/java/org/apache/tez/runtime/library/common/shuffle/impl/TestShuffleManager.java:
##########
Review Comment:
done
--
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: issues-unsubscribe@tez.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] TEZ-4757: APPROXIMATE_INPUT_RECORDS over-counts the rows an input will deliver [tez]
Posted by "deniskuzZ (via GitHub)" <gi...@apache.org>.
deniskuzZ commented on code in PR #540:
URL: https://github.com/apache/tez/pull/540#discussion_r4144900030
##########
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:
changed
--
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: issues-unsubscribe@tez.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] TEZ-4757: APPROXIMATE_INPUT_RECORDS over-counts the rows an input will deliver [tez]
Posted by "abstractdog (via GitHub)" <gi...@apache.org>.
abstractdog commented on code in PR #540:
URL: https://github.com/apache/tez/pull/540#discussion_r4143457528
##########
tez-runtime-library/src/test/java/org/apache/tez/runtime/library/common/writers/TestUnorderedPartitionedKVWriter.java:
##########
@@ -432,6 +433,72 @@ public void testTextMixedRecordsWithoutFinalMerge(boolean shouldCompress, Report
textTest(100, 10, 2048, 10, 10, 10, false, false);
}
+ /**
+ * One partition with pipelined shuffle is the only shape where writeLargeRecord runs while
+ * num_record is set, so it is the only one that can show the DME under-counting large records.
+ */
+ @ParameterizedTest(name = "test[{0}, {1}]")
+ @MethodSource("data")
+ @Timeout(value = 10000, unit = TimeUnit.MILLISECONDS)
+ public void testLargeRecords_SinglePartition(boolean shouldCompress,
+ ReportPartitionStats reportPartitionStats) throws IOException, InterruptedException {
+ setupInit(shouldCompress, reportPartitionStats);
+ textTest(0, 1, 2048, 0, 0, 5, true, false);
+ }
+
+ /**
+ * An output small enough to fit one buffer spills only at close, so its records reach the task
+ * counters in the same call that builds the final event -- which is where a count read too
+ * early reports zero.
+ */
+ @ParameterizedTest(name = "test[{0}, {1}]")
+ @MethodSource("data")
+ @Timeout(value = 10000, unit = TimeUnit.MILLISECONDS)
+ public void testFinalEventRecordCount_SinglePartition(boolean shouldCompress,
Review Comment:
see https://github.com/apache/tez/pull/540/changes#r4143456360
##########
tez-runtime-library/src/test/java/org/apache/tez/runtime/library/common/writers/TestUnorderedPartitionedKVWriter.java:
##########
@@ -432,6 +433,72 @@ public void testTextMixedRecordsWithoutFinalMerge(boolean shouldCompress, Report
textTest(100, 10, 2048, 10, 10, 10, false, false);
}
+ /**
+ * One partition with pipelined shuffle is the only shape where writeLargeRecord runs while
+ * num_record is set, so it is the only one that can show the DME under-counting large records.
+ */
+ @ParameterizedTest(name = "test[{0}, {1}]")
+ @MethodSource("data")
+ @Timeout(value = 10000, unit = TimeUnit.MILLISECONDS)
+ public void testLargeRecords_SinglePartition(boolean shouldCompress,
Review Comment:
I think checkstyle will report this because of the underscore, let's make it happy by simply writing `testLargeRecordsSinglePartition` (even if I know that using underscore like this in unit test names is valid)
##########
tez-runtime-library/src/test/java/org/apache/tez/runtime/library/common/writers/TestUnorderedPartitionedKVWriter.java:
##########
@@ -432,6 +433,72 @@ public void testTextMixedRecordsWithoutFinalMerge(boolean shouldCompress, Report
textTest(100, 10, 2048, 10, 10, 10, false, false);
}
+ /**
+ * One partition with pipelined shuffle is the only shape where writeLargeRecord runs while
+ * num_record is set, so it is the only one that can show the DME under-counting large records.
+ */
+ @ParameterizedTest(name = "test[{0}, {1}]")
+ @MethodSource("data")
+ @Timeout(value = 10000, unit = TimeUnit.MILLISECONDS)
+ public void testLargeRecords_SinglePartition(boolean shouldCompress,
+ ReportPartitionStats reportPartitionStats) throws IOException, InterruptedException {
+ setupInit(shouldCompress, reportPartitionStats);
+ textTest(0, 1, 2048, 0, 0, 5, true, false);
+ }
+
+ /**
+ * An output small enough to fit one buffer spills only at close, so its records reach the task
+ * counters in the same call that builds the final event -- which is where a count read too
+ * early reports zero.
+ */
+ @ParameterizedTest(name = "test[{0}, {1}]")
+ @MethodSource("data")
+ @Timeout(value = 10000, unit = TimeUnit.MILLISECONDS)
+ public void testFinalEventRecordCount_SinglePartition(boolean shouldCompress,
+ ReportPartitionStats reportPartitionStats) throws IOException, InterruptedException {
+ setupInit(shouldCompress, reportPartitionStats);
+ textTest(5, 1, 2048, 0, 0, 0, true, false);
+ }
+
+ /**
+ * A single-partition writer is the only one that fills num_record, and large records never
+ * reach outputRecordsCounter -- writeLargeRecord bypasses it -- so both the DME and the
+ * VertexManager event have to carry the sum of the two counters. Under pipelined shuffle the
+ * events are pushed through sendEvents rather than returned by close(), so both sources are
+ * scanned and the last of each kind is the one that has to be right.
+ */
+ private void assertLastEventsCountEveryRecord(UnorderedPartitionedKVWriter kvWriter,
+ OutputContext outputContext, List<Event> closeEvents) throws IOException {
+ List<Event> allEvents = new ArrayList<>(closeEvents);
+ @SuppressWarnings("unchecked")
+ ArgumentCaptor<List<Event>> sent = ArgumentCaptor.forClass(List.class);
+ verify(outputContext, atLeast(0)).sendEvents(sent.capture());
+ sent.getAllValues().forEach(allEvents::addAll);
+
+ long expected = kvWriter.outputRecordsCounter.getValue()
+ + kvWriter.outputLargeRecordsCounter.getValue();
Review Comment:
isn't the ground truth somehow derived from `numRecordsWritten`, which is closer to reality? or assert against both `expected` and `numRecordsWritten`
##########
tez-runtime-library/src/test/java/org/apache/tez/runtime/library/common/shuffle/impl/TestShuffleInputEventHandlerImpl.java:
##########
@@ -408,6 +408,63 @@ public void testDataMovementEventsWithShuffleData() throws IOException {
}
}
+ /**
+ * An input whose partition is empty reports zero rows and must still reach the ShuffleManager:
+ * counting it only in numInputs and not in the denominator is what inflated the counter.
+ */
+ @Test
+ @Timeout(value = 5000, unit = TimeUnit.MILLISECONDS)
+ public void testAnEmptyPartitionStillReportsItsRecordCount() throws IOException {
+ ShuffleManager shuffleManager = mock(ShuffleManager.class);
+ ShuffleInputEventHandlerImpl handler = newHandler(shuffleManager, false);
+
+ handler.handleEvents(Collections.singletonList(
+ createRecordCountEvent(1, 0, createEmptyPartitionByteString(0), false)));
+
+ verify(shuffleManager).updateApproximateInputRecords(eq(1), eq(0L));
+ verify(shuffleManager).addCompletedInputWithNoData(any());
+ }
+
+ /** The composite path carries the same payload and has to report it too. */
+ @Test
+ @Timeout(value = 5000, unit = TimeUnit.MILLISECONDS)
+ public void testACompositeEventReportsItsRecordCount() throws IOException {
+ ShuffleManager shuffleManager = mock(ShuffleManager.class);
+ ShuffleInputEventHandlerImpl handler = newHandler(shuffleManager, true);
+
+ handler.handleEvents(Collections.singletonList(createRecordCountEvent(1, 4000, null, true)));
+
+ verify(shuffleManager).updateApproximateInputRecords(eq(1), eq(4000L));
+ }
+
+ /**
+ * A source task can emit more rows than an int32 holds. num_record is an int64 so the count
+ * survives the wire; passing it as an int would not even compile against that field.
+ */
+ @Test
+ @Timeout(value = 5000, unit = TimeUnit.MILLISECONDS)
+ public void testACountAboveIntMaxSurvivesTheWire() throws IOException {
+ ShuffleManager shuffleManager = mock(ShuffleManager.class);
+ ShuffleInputEventHandlerImpl handler = newHandler(shuffleManager, false);
+
+ handler.handleEvents(Collections.singletonList(
+ createRecordCountEvent(1, 3_000_000_000L, null, false)));
+
+ verify(shuffleManager).updateApproximateInputRecords(eq(1), eq(3_000_000_000L));
+ }
+
+ /** A payload with no record count -- any multi-partition writer -- reports nothing. */
+ @Test
+ @Timeout(value = 5000, unit = TimeUnit.MILLISECONDS)
+ public void testAPayloadWithoutARecordCountReportsNothing() throws IOException {
+ ShuffleManager shuffleManager = mock(ShuffleManager.class);
+ ShuffleInputEventHandlerImpl handler = newHandler(shuffleManager, false);
+
+ handler.handleEvents(Collections.singletonList(createDataMovementEvent(0, 1, null)));
+
+ verify(shuffleManager, times(0)).updateApproximateInputRecords(anyInt(), anyLong());
+ }
Review Comment:
while I understand the goal of this unit test, nothing strongly shows a multi-partition writer here, which is confusing, its real goal would be clearer by having said like something below:
```
/**
* A payload that carries no record count reports nothing: the unset field is not read as a
* zero, which would count the input as having written no rows.
*/
```
##########
tez-runtime-library/src/test/java/org/apache/tez/runtime/library/common/writers/TestUnorderedPartitionedKVWriter.java:
##########
@@ -432,6 +433,72 @@ public void testTextMixedRecordsWithoutFinalMerge(boolean shouldCompress, Report
textTest(100, 10, 2048, 10, 10, 10, false, false);
}
+ /**
+ * One partition with pipelined shuffle is the only shape where writeLargeRecord runs while
+ * num_record is set, so it is the only one that can show the DME under-counting large records.
+ */
+ @ParameterizedTest(name = "test[{0}, {1}]")
+ @MethodSource("data")
+ @Timeout(value = 10000, unit = TimeUnit.MILLISECONDS)
+ public void testLargeRecords_SinglePartition(boolean shouldCompress,
+ ReportPartitionStats reportPartitionStats) throws IOException, InterruptedException {
+ setupInit(shouldCompress, reportPartitionStats);
+ textTest(0, 1, 2048, 0, 0, 5, true, false);
+ }
+
+ /**
+ * An output small enough to fit one buffer spills only at close, so its records reach the task
+ * counters in the same call that builds the final event -- which is where a count read too
+ * early reports zero.
Review Comment:
"which is where a count read too early reports zero", this feels like mentioning the pre-patch state, could it be improved to focus on what the unit test actually pins?
##########
tez-runtime-library/src/test/java/org/apache/tez/runtime/library/common/writers/TestUnorderedPartitionedKVWriter.java:
##########
@@ -432,6 +433,72 @@ public void testTextMixedRecordsWithoutFinalMerge(boolean shouldCompress, Report
textTest(100, 10, 2048, 10, 10, 10, false, false);
}
+ /**
+ * One partition with pipelined shuffle is the only shape where writeLargeRecord runs while
+ * num_record is set, so it is the only one that can show the DME under-counting large records.
+ */
+ @ParameterizedTest(name = "test[{0}, {1}]")
+ @MethodSource("data")
+ @Timeout(value = 10000, unit = TimeUnit.MILLISECONDS)
+ public void testLargeRecords_SinglePartition(boolean shouldCompress,
+ ReportPartitionStats reportPartitionStats) throws IOException, InterruptedException {
+ setupInit(shouldCompress, reportPartitionStats);
+ textTest(0, 1, 2048, 0, 0, 5, true, false);
+ }
+
+ /**
+ * An output small enough to fit one buffer spills only at close, so its records reach the task
+ * counters in the same call that builds the final event -- which is where a count read too
+ * early reports zero.
+ */
+ @ParameterizedTest(name = "test[{0}, {1}]")
+ @MethodSource("data")
+ @Timeout(value = 10000, unit = TimeUnit.MILLISECONDS)
+ public void testFinalEventRecordCount_SinglePartition(boolean shouldCompress,
+ ReportPartitionStats reportPartitionStats) throws IOException, InterruptedException {
+ setupInit(shouldCompress, reportPartitionStats);
+ textTest(5, 1, 2048, 0, 0, 0, true, false);
+ }
+
+ /**
+ * A single-partition writer is the only one that fills num_record, and large records never
+ * reach outputRecordsCounter -- writeLargeRecord bypasses it -- so both the DME and the
+ * VertexManager event have to carry the sum of the two counters. Under pipelined shuffle the
+ * events are pushed through sendEvents rather than returned by close(), so both sources are
+ * scanned and the last of each kind is the one that has to be right.
+ */
+ private void assertLastEventsCountEveryRecord(UnorderedPartitionedKVWriter kvWriter,
+ OutputContext outputContext, List<Event> closeEvents) throws IOException {
+ List<Event> allEvents = new ArrayList<>(closeEvents);
+ @SuppressWarnings("unchecked")
+ ArgumentCaptor<List<Event>> sent = ArgumentCaptor.forClass(List.class);
+ verify(outputContext, atLeast(0)).sendEvents(sent.capture());
+ sent.getAllValues().forEach(allEvents::addAll);
+
+ long expected = kvWriter.outputRecordsCounter.getValue()
+ + kvWriter.outputLargeRecordsCounter.getValue();
+ long lastDmeCount = -1;
+ long lastVmCount = -1;
+ for (Event event : allEvents) {
+ if (event instanceof CompositeDataMovementEvent) {
+ ShuffleUserPayloads.DataMovementEventPayloadProto dme =
+ ShuffleUserPayloads.DataMovementEventPayloadProto.parseFrom(ByteString.copyFrom(
+ ((CompositeDataMovementEvent) event).getUserPayload()));
+ if (dme.hasNumRecord()) {
+ lastDmeCount = dme.getNumRecord();
+ }
+ } else if (event instanceof VertexManagerEvent) {
+ ShuffleUserPayloads.VertexManagerEventPayloadProto vme =
+ ShuffleUserPayloads.VertexManagerEventPayloadProto.parseFrom(ByteString.copyFrom(
+ ((VertexManagerEvent) event).getUserPayload()));
+ lastVmCount = vme.getNumRecord();
+ }
+ }
+ assertTrue(lastDmeCount >= 0, "expected a DME carrying num_record");
+ assertEquals(expected, lastDmeCount, "DME");
+ assertEquals(expected, lastVmCount, "VertexManagerEvent");
Review Comment:
please improve assertion messages here
##########
tez-runtime-library/src/test/java/org/apache/tez/runtime/library/common/shuffle/TestShuffleUtils.java:
##########
@@ -286,6 +286,36 @@ public void testGenerateOnSpillEvent_With_All_EmptyPartitions() throws Exception
"emptyPartitionBitSet cardinality (expecting 10) = " + emptyPartitionsBitSet.cardinality());
}
+ /**
+ * An output that never started delivers no rows, and a single-partition one fills num_record
+ * like any other, so it joins the consumer's denominator instead of being scaled over.
+ */
+ @Test
+ public void testNonStartedSinglePartitionOutputReportsZeroRecords() throws Exception {
+ List<Event> events = Lists.newLinkedList();
+ ShuffleUtils.generateEventsForNonStartedOutput(events, 1, outputContext, false, true,
+ TezCommonUtils.newBestCompressionDeflater());
+
+ ShuffleUserPayloads.DataMovementEventPayloadProto proto =
Review Comment:
`DataMovementEventPayloadProto` is already imported, I would use it instead
##########
tez-runtime-library/src/test/java/org/apache/tez/runtime/library/common/writers/TestUnorderedPartitionedKVWriter.java:
##########
@@ -432,6 +433,72 @@ public void testTextMixedRecordsWithoutFinalMerge(boolean shouldCompress, Report
textTest(100, 10, 2048, 10, 10, 10, false, false);
}
+ /**
Review Comment:
maybe a missing test case, single partition, no data, pipeline shuffle:
```
textTest(0, 1, 2048, 0, 0, 0, true, false)
```
covering the "all empty partitions" case, look at the `writer.finalSpill` method
--
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: issues-unsubscribe@tez.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] TEZ-4757: APPROXIMATE_INPUT_RECORDS over-counts the rows an input will deliver [tez]
Posted by "deniskuzZ (via GitHub)" <gi...@apache.org>.
deniskuzZ commented on code in PR #540:
URL: https://github.com/apache/tez/pull/540#discussion_r4144659241
##########
tez-runtime-library/src/test/java/org/apache/tez/runtime/library/common/writers/TestUnorderedPartitionedKVWriter.java:
##########
@@ -432,6 +433,72 @@ public void testTextMixedRecordsWithoutFinalMerge(boolean shouldCompress, Report
textTest(100, 10, 2048, 10, 10, 10, false, false);
}
+ /**
+ * One partition with pipelined shuffle is the only shape where writeLargeRecord runs while
+ * num_record is set, so it is the only one that can show the DME under-counting large records.
+ */
+ @ParameterizedTest(name = "test[{0}, {1}]")
+ @MethodSource("data")
+ @Timeout(value = 10000, unit = TimeUnit.MILLISECONDS)
+ public void testLargeRecords_SinglePartition(boolean shouldCompress,
Review Comment:
renamed
--
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: issues-unsubscribe@tez.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] TEZ-4757: APPROXIMATE_INPUT_RECORDS over-counts the rows an input will deliver [tez]
Posted by "deniskuzZ (via GitHub)" <gi...@apache.org>.
deniskuzZ commented on code in PR #540:
URL: https://github.com/apache/tez/pull/540#discussion_r4144728476
##########
tez-runtime-library/src/test/java/org/apache/tez/runtime/library/common/shuffle/TestShuffleUtils.java:
##########
@@ -286,6 +286,36 @@ public void testGenerateOnSpillEvent_With_All_EmptyPartitions() throws Exception
"emptyPartitionBitSet cardinality (expecting 10) = " + emptyPartitionsBitSet.cardinality());
}
+ /**
+ * An output that never started delivers no rows, and a single-partition one fills num_record
+ * like any other, so it joins the consumer's denominator instead of being scaled over.
+ */
+ @Test
+ public void testNonStartedSinglePartitionOutputReportsZeroRecords() throws Exception {
+ List<Event> events = Lists.newLinkedList();
+ ShuffleUtils.generateEventsForNonStartedOutput(events, 1, outputContext, false, true,
+ TezCommonUtils.newBestCompressionDeflater());
+
+ ShuffleUserPayloads.DataMovementEventPayloadProto proto =
Review Comment:
done
--
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: issues-unsubscribe@tez.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] TEZ-4757: APPROXIMATE_INPUT_RECORDS over-counts the rows an input will deliver [tez]
Posted by "deniskuzZ (via GitHub)" <gi...@apache.org>.
deniskuzZ commented on code in PR #540:
URL: https://github.com/apache/tez/pull/540#discussion_r4144889086
##########
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:
done
--
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: issues-unsubscribe@tez.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org