Skip to content

Conversation

lnbest0707-uber
Copy link
Contributor

@lnbest0707-uber lnbest0707-uber commented Aug 26, 2025

real-time ingestion feature
Part 2 of #15782, but this is independent from part 1
Issue #14815
Design doc https://docs.google.com/document/d/1NKPeNh6V2ctaQ4T_X3OKJ6Gcy5TRanLiU1uIDT8_9UA/edit?usp=sharing
This is an alternative PR of #16494

Instead of introducing the new segment naming format that concerns some parties, this PR chooses the way to "soft delete" the topics by marking it as 'inactive'.
We still cannot literally delete a topic from the table config.
But instead, we would introduce a similar "pause" consumption API to mark topic as 'inactive' in the idealstae. As a result, the consumption of the topic would stop.

This 'inactive' marking would also prevent the metadata being queries from the stream. So that it would be safe to delete the topic from the stream.

The table idealstate could look like (with pauseState field an additional key added)

{
  "id": "logging_palantir_raw_REALTIME",
  "simpleFields": {
    "BATCH_MESSAGE_MODE": "false",
    "IDEAL_STATE_MODE": "CUSTOMIZED",
    "INSTANCE_GROUP_TAG": "logging_palantir_raw_REALTIME",
    "MAX_PARTITIONS_PER_INSTANCE": "1",
    "NUM_PARTITIONS": "1102",
    "REBALANCE_MODE": "CUSTOMIZED",
    "REPLICAS": "2",
    "STATE_MODEL_DEF_REF": "SegmentOnlineOfflineStateModel",
    "STATE_MODEL_FACTORY_NAME": "DEFAULT",
    "enableCompression": "true",
    "isTablePaused": "false",
    "pauseState": "{\"reasonCode\":\"ADMINISTRATIVE\",\"timeInMillis\":\"2025-08-26 18:11:47.816\",\"paused\":false, \"indexOfInActiveTopics\": [1, 2]}"
  },
  "mapFields": {
     ...
  }
}

@codecov-commenter
Copy link

codecov-commenter commented Aug 26, 2025

Codecov Report

❌ Patch coverage is 20.17544% with 91 lines in your changes missing coverage. Please review.
✅ Project coverage is 63.36%. Comparing base (777936a) to head (0907419).
⚠️ Report is 13 commits behind head on master.

Files with missing lines Patch % Lines
.../core/realtime/PinotLLCRealtimeSegmentManager.java 18.57% 53 Missing and 4 partials ⚠️
...ller/api/resources/PinotRealtimeTableResource.java 0.00% 28 Missing ⚠️
.../org/apache/pinot/spi/config/table/PauseState.java 0.00% 5 Missing ⚠️
...x/core/realtime/MissingConsumingSegmentFinder.java 75.00% 0 Missing and 1 partial ⚠️
Additional details and impacted files
@@             Coverage Diff              @@
##             master   #16692      +/-   ##
============================================
+ Coverage     63.33%   63.36%   +0.02%     
- Complexity     1379     1380       +1     
============================================
  Files          3036     3036              
  Lines        176785   177006     +221     
  Branches      27131    27150      +19     
============================================
+ Hits         111971   112152     +181     
- Misses        56227    56248      +21     
- Partials       8587     8606      +19     
Flag Coverage Δ
custom-integration1 100.00% <ø> (ø)
integration 100.00% <ø> (ø)
integration1 100.00% <ø> (ø)
integration2 0.00% <ø> (?)
java-11 63.32% <20.17%> (+0.03%) ⬆️
java-21 63.33% <20.17%> (+0.01%) ⬆️
temurin 63.36% <20.17%> (+0.02%) ⬆️
unittests 63.35% <20.17%> (+0.02%) ⬆️
unittests1 56.56% <54.54%> (+0.13%) ⬆️
unittests2 33.10% <16.66%> (-0.06%) ⬇️

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.

@lnbest0707-uber lnbest0707-uber changed the title [Auto reset 2/3 - alternative]Introduce topic 'inactive' status [Auto reset 2/3]Introduce topic 'inactive' status Aug 28, 2025
@chenboat
Copy link
Contributor

@lnbest0707-uber can you check if the test failures are related?

@lnbest0707-uber
Copy link
Contributor Author

@lnbest0707-uber can you check if the test failures are related?

Pinot Binary Compatibility Check (pull_request) is due to changing the constructor. But since we will use the new and discontinue using the old. I feel I should remove the old instead keeping both.

The integration test succeeds on Java21 and should be flaky.

@chenboat chenboat merged commit c44fd79 into apache:master Aug 29, 2025
16 of 18 checks passed
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.

4 participants