Skip to content

Adds Intgeration tests for Dedup and Pauseless#15398

Merged
KKcorps merged 7 commits intoapache:masterfrom
noob-se7en:integration_test
May 2, 2025
Merged

Adds Intgeration tests for Dedup and Pauseless#15398
KKcorps merged 7 commits intoapache:masterfrom
noob-se7en:integration_test

Conversation

@noob-se7en
Copy link
Contributor

Adds Intgeration tests for Dedup and Pauseless enforcing consumption in order.

@noob-se7en noob-se7en marked this pull request as draft March 28, 2025 12:39
@codecov-commenter
Copy link

codecov-commenter commented Mar 28, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 62.81%. Comparing base (2d8d8c3) to head (5f060b2).

Additional details and impacted files
@@             Coverage Diff              @@
##             master   #15398      +/-   ##
============================================
- Coverage     62.82%   62.81%   -0.01%     
  Complexity     1384     1384              
============================================
  Files          2864     2864              
  Lines        162671   162671              
  Branches      24902    24902              
============================================
- Hits         102191   102189       -2     
- Misses        52782    52788       +6     
+ Partials       7698     7694       -4     
Flag Coverage Δ
custom-integration1 100.00% <ø> (ø)
integration 100.00% <ø> (ø)
integration1 100.00% <ø> (ø)
integration2 0.00% <ø> (ø)
java-11 62.77% <ø> (+<0.01%) ⬆️
java-21 62.80% <ø> (-0.01%) ⬇️
skip-bytebuffers-false 62.79% <ø> (-0.02%) ⬇️
skip-bytebuffers-true 62.78% <ø> (+0.01%) ⬆️
temurin 62.81% <ø> (-0.01%) ⬇️
unittests 62.81% <ø> (-0.01%) ⬇️
unittests1 55.76% <ø> (-0.02%) ⬇️
unittests2 33.52% <ø> (-0.01%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@noob-se7en noob-se7en marked this pull request as ready for review April 21, 2025 12:53
@KKcorps KKcorps requested a review from Copilot April 23, 2025 09:02
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR adds integration tests for deduplication and pauseless real‐time ingestion, enforcing consumption in order. Key changes include:

  • Introducing a new test (PauselessRealtimeIngestionWithDedupIntegrationTest) that enables pauseless consumption.
  • Updating the ingestion configuration across multiple test files to enforce consumption order.
  • Refactoring builder chains for consistency in table configuration construction.

Reviewed Changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated no comments.

File Description
pinot-integration-tests/src/test/java/org/apache/pinot/integration/tests/PauselessRealtimeIngestionWithDedupIntegrationTest.java Adds a new integration test that sets pauseless consumption enabled on the ingestion configuration.
pinot-integration-tests/src/test/java/org/apache/pinot/integration/tests/DedupPreloadIntegrationTest.java Updates ingestion configuration and reformats the table config builder chain.
pinot-integration-tests/src/test/java/org/apache/pinot/integration/tests/BaseDedupIntegrationTest.java Adds an override for getIngestionConfig() and updates the table configuration builder chain.
pinot-integration-test-base/src/test/java/org/apache/pinot/integration/tests/BaseClusterIntegrationTest.java Refactors the table config builder chain to match the new formatting style and ingestion config changes.

IngestionConfig ingestionConfig = super.getIngestionConfig();
assert ingestionConfig != null;
assert ingestionConfig.getStreamIngestionConfig() != null;
ingestionConfig.getStreamIngestionConfig().setPauselessConsumptionEnabled(true);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we not enable rest of the pauseless + dedup configs here like parallel consumption policy?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

They will be enabled from line 27.
IngestionConfig ingestionConfig = super.getIngestionConfig();

@noob-se7en noob-se7en requested a review from KKcorps May 2, 2025 12:44
@KKcorps KKcorps merged commit 55eb236 into apache:master May 2, 2025
22 checks passed
@ankitsultana
Copy link
Contributor

@KKcorps @noob-se7en : has this broken tests on master?

Seeing this in my PR:

Error:  Failures: 
Error:    PauselessRealtimeIngestionWithDedupIntegrationTest>BaseDedupIntegrationTest.setUp:79->BaseDedupIntegrationTest.createDedupConfigsWithReplicas:89->ControllerTest.addTableConfig:694 » IO org.apache.pinot.common.exception.HttpErrorStatusException: Got error status code: 400 (Bad Request) with reason: "Must have a valid peerSegmentDownloadScheme set in validation config for pauseless consumption" while sending request: /tables to controller: fv-az2246-474.ij2tcco034yevoccl5q4x054ic.cx.internal.cloudapp.net, version: 1.4.0-SNAPSHOT-e03df223ba21767b388ac1ec3561cc5d3d5e9c50

@KKcorps
Copy link
Contributor

KKcorps commented May 2, 2025

@ankitsultana that is due to a different PR

#15567

@KKcorps
Copy link
Contributor

KKcorps commented May 2, 2025

@noob-se7en can you please fix this test to address the error

@noob-se7en
Copy link
Contributor Author

yep just saw as well

@noob-se7en
Copy link
Contributor Author

#15699

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants