You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@pinot.apache.org by "J-HowHuang (via GitHub)" <gi...@apache.org> on 2026/05/13 19:09:30 UTC
[PR] Add a latch to allow ingestion pause during startup [pinot]
J-HowHuang opened a new pull request, #18492:
URL: https://github.com/apache/pinot/pull/18492
## Description
Depends on use cases, server starter might need to pause ingestion during start up for a better performance.
## Changes
### New boolean supplier for ingestion readiness
Add a new boolean supplier `boolean isIngestionPausedDueToStartUp()` in `BaseServerStarter` that will be hooked into `TableDataManagerProvider` to construct `RealtimeTableDataManager`. This new boolean supplier, and the existing boolean supplier in `RealtimeTableDataManager` that signals whether it's ready to ingest on dedup or paritial upsert table, they together will determine if the ingestion loop should start in `RealtimeSegmentDataManager`
### Add metrics to track time-to-ready for individual server start up status checks
* `STARTUP_CURRENT_STATE_MATCH_TIME_MS`
* `STARTUP_EXTERNAL_VIEW_MATCH_TIME_MS`
* `STARTUP_REALTIME_CONSUMPTION_CATCHUP_TIME_MS`
These gauge will show the time spent on individual phase of the server start up service status checks.
They are tracked by a new class `TimeToHealthyTrackingCallback`, which is a wrapper around the original `ServiceStatusCallback` that track the time and emit the metrics.
--
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: commits-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
Re: [PR] Add a latch to allow ingestion pause during startup [pinot]
Posted by "J-HowHuang (via GitHub)" <gi...@apache.org>.
J-HowHuang commented on code in PR #18492:
URL: https://github.com/apache/pinot/pull/18492#discussion_r3238275977
##########
pinot-core/src/main/java/org/apache/pinot/core/data/manager/provider/DefaultTableDataManagerProvider.java:
##########
@@ -75,8 +75,8 @@ public TableDataManager getTableDataManager(TableConfig tableConfig, Schema sche
SegmentReloadSemaphore segmentReloadSemaphore, ExecutorService segmentReloadRefreshExecutor,
@Nullable ExecutorService segmentPreloadExecutor,
@Nullable Cache<Pair<String, String>, SegmentErrorInfo> errorCache,
- BooleanSupplier isServerReadyToServeQueries, boolean enableAsyncSegmentRefresh,
- ServerReloadJobStatusCache reloadJobStatusCache) {
+ BooleanSupplier isServerReadyToServeQueries, BooleanSupplier isIngestionPausedDueToStartUp,
Review Comment:
Re-modeled to `isServerReadyToConsumeData` for all occurrences. also re-ordered in the code and argument positions.
--
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: commits-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
Re: [PR] Add a latch to allow ingestion pause during startup [pinot]
Posted by "codecov-commenter (via GitHub)" <gi...@apache.org>.
codecov-commenter commented on PR #18492:
URL: https://github.com/apache/pinot/pull/18492#issuecomment-4445479864
## [Codecov](https://app.codecov.io/gh/apache/pinot/pull/18492?dropdown=coverage&src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) Report
:x: Patch coverage is `63.63636%` with `4 lines` in your changes missing coverage. Please review.
:white_check_mark: Project coverage is 55.81%. Comparing base ([`c35dee5`](https://app.codecov.io/gh/apache/pinot/commit/c35dee598c2d312ed5fa9396e0ffe8ab99fd1e3b?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)) to head ([`a81ee26`](https://app.codecov.io/gh/apache/pinot/commit/a81ee2616c593d9d6143e1ffa31479898cfa8097?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)).
:warning: Report is 6 commits behind head on master.
| [Files with missing lines](https://app.codecov.io/gh/apache/pinot/pull/18492?dropdown=coverage&src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | Patch % | Lines |
|---|---|---|
| [...ata/manager/realtime/RealtimeTableDataManager.java](https://app.codecov.io/gh/apache/pinot/pull/18492?src=pr&el=tree&filepath=pinot-core%2Fsrc%2Fmain%2Fjava%2Forg%2Fapache%2Fpinot%2Fcore%2Fdata%2Fmanager%2Frealtime%2FRealtimeTableDataManager.java&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9kYXRhL21hbmFnZXIvcmVhbHRpbWUvUmVhbHRpbWVUYWJsZURhdGFNYW5hZ2VyLmphdmE=) | 62.50% | [3 Missing :warning: ](https://app.codecov.io/gh/apache/pinot/pull/18492?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
| [...e/pinot/core/data/manager/InstanceDataManager.java](https://app.codecov.io/gh/apache/pinot/pull/18492?src=pr&el=tree&filepath=pinot-core%2Fsrc%2Fmain%2Fjava%2Forg%2Fapache%2Fpinot%2Fcore%2Fdata%2Fmanager%2FInstanceDataManager.java&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9kYXRhL21hbmFnZXIvSW5zdGFuY2VEYXRhTWFuYWdlci5qYXZh) | 0.00% | [1 Missing :warning: ](https://app.codecov.io/gh/apache/pinot/pull/18492?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
> :exclamation: There is a different number of reports uploaded between BASE (c35dee5) and HEAD (a81ee26). Click for more details.
>
> <details><summary>HEAD has 16 uploads less than BASE</summary>
>
>| Flag | BASE (c35dee5) | HEAD (a81ee26) |
>|------|------|------|
>|java-21|5|1|
>|unittests|2|1|
>|temurin|5|1|
>|unittests2|1|0|
>|integration|3|0|
>|integration1|1|0|
>|custom-integration1|1|0|
>|integration2|1|0|
></details>
<details><summary>Additional details and impacted files</summary>
```diff
@@ Coverage Diff @@
## master #18492 +/- ##
============================================
- Coverage 63.68% 55.81% -7.88%
+ Complexity 1685 816 -869
============================================
Files 3265 2556 -709
Lines 199745 148284 -51461
Branches 31013 23950 -7063
============================================
- Hits 127215 82759 -44456
+ Misses 62395 58441 -3954
+ Partials 10135 7084 -3051
```
| [Flag](https://app.codecov.io/gh/apache/pinot/pull/18492/flags?src=pr&el=flags&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | Coverage Δ | |
|---|---|---|
| [custom-integration1](https://app.codecov.io/gh/apache/pinot/pull/18492/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `?` | |
| [integration](https://app.codecov.io/gh/apache/pinot/pull/18492/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `?` | |
| [integration1](https://app.codecov.io/gh/apache/pinot/pull/18492/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `?` | |
| [integration2](https://app.codecov.io/gh/apache/pinot/pull/18492/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `?` | |
| [java-21](https://app.codecov.io/gh/apache/pinot/pull/18492/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `55.81% <63.63%> (-7.88%)` | :arrow_down: |
| [temurin](https://app.codecov.io/gh/apache/pinot/pull/18492/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `55.81% <63.63%> (-7.88%)` | :arrow_down: |
| [unittests](https://app.codecov.io/gh/apache/pinot/pull/18492/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `55.81% <63.63%> (-7.88%)` | :arrow_down: |
| [unittests1](https://app.codecov.io/gh/apache/pinot/pull/18492/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `55.81% <63.63%> (+0.03%)` | :arrow_up: |
| [unittests2](https://app.codecov.io/gh/apache/pinot/pull/18492/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `?` | |
Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#carryforward-flags-in-the-pull-request-comment) to find out more.
</details>
[:umbrella: View full report in Codecov by Sentry](https://app.codecov.io/gh/apache/pinot/pull/18492?dropdown=coverage&src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache).
:loudspeaker: Have feedback on the report? [Share it here](https://about.codecov.io/codecov-pr-comment-feedback/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache).
<details><summary> :rocket: New features to boost your workflow: </summary>
- :package: [JS Bundle Analysis](https://docs.codecov.com/docs/javascript-bundle-analysis): Save yourself from yourself by tracking and limiting bundle sizes in JS merges.
</details>
--
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: commits-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
Re: [PR] Add a latch to allow ingestion pause during startup [pinot]
Posted by "J-HowHuang (via GitHub)" <gi...@apache.org>.
J-HowHuang commented on code in PR #18492:
URL: https://github.com/apache/pinot/pull/18492#discussion_r3238277836
##########
pinot-core/src/main/java/org/apache/pinot/core/data/manager/provider/TableDataManagerProvider.java:
##########
@@ -55,15 +55,34 @@ TableDataManager getTableDataManager(TableConfig tableConfig,
@Nullable ExecutorService segmentPreloadExecutor,
@Nullable Cache<Pair<String, String>, SegmentErrorInfo> errorCache,
BooleanSupplier isServerReadyToServeQueries,
+ BooleanSupplier isIngestionPausedDueToStartUp,
boolean enableAsyncSegmentRefresh,
ServerReloadJobStatusCache reloadJobStatusCache);
+ /**
+ * Backward-compatible overload that does not gate ingestion at startup. Delegates to the full overload with
+ * {@code isIngestionPausedDueToStartUp = () -> false}.
+ */
+ default TableDataManager getTableDataManager(TableConfig tableConfig,
Review Comment:
Removed the fall-back constructor
##########
pinot-core/src/main/java/org/apache/pinot/core/data/manager/realtime/RealtimeTableDataManager.java:
##########
@@ -138,6 +138,7 @@ public class RealtimeTableDataManager extends BaseTableDataManager {
protected Cache<String, StreamMetadataProvider> _streamMetadataProviderCache;
private final BooleanSupplier _isServerReadyToServeQueries;
+ private final BooleanSupplier _isIngestionPausedDueToStartUp;
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: commits-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
Re: [PR] Add a latch to allow ingestion pause during startup [pinot]
Posted by "Jackie-Jiang (via GitHub)" <gi...@apache.org>.
Jackie-Jiang merged PR #18492:
URL: https://github.com/apache/pinot/pull/18492
--
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: commits-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
Re: [PR] Add a latch to allow ingestion pause during startup [pinot]
Posted by "Jackie-Jiang (via GitHub)" <gi...@apache.org>.
Jackie-Jiang commented on code in PR #18492:
URL: https://github.com/apache/pinot/pull/18492#discussion_r3238043607
##########
pinot-core/src/main/java/org/apache/pinot/core/data/manager/provider/TableDataManagerProvider.java:
##########
@@ -55,15 +55,34 @@ TableDataManager getTableDataManager(TableConfig tableConfig,
@Nullable ExecutorService segmentPreloadExecutor,
@Nullable Cache<Pair<String, String>, SegmentErrorInfo> errorCache,
BooleanSupplier isServerReadyToServeQueries,
+ BooleanSupplier isIngestionPausedDueToStartUp,
boolean enableAsyncSegmentRefresh,
ServerReloadJobStatusCache reloadJobStatusCache);
+ /**
+ * Backward-compatible overload that does not gate ingestion at startup. Delegates to the full overload with
+ * {@code isIngestionPausedDueToStartUp = () -> false}.
+ */
+ default TableDataManager getTableDataManager(TableConfig tableConfig,
Review Comment:
Directly modify the main constructor. This is an internal only interface, no need to keep backward compatibility
##########
pinot-core/src/main/java/org/apache/pinot/core/data/manager/provider/DefaultTableDataManagerProvider.java:
##########
@@ -75,8 +75,8 @@ public TableDataManager getTableDataManager(TableConfig tableConfig, Schema sche
SegmentReloadSemaphore segmentReloadSemaphore, ExecutorService segmentReloadRefreshExecutor,
@Nullable ExecutorService segmentPreloadExecutor,
@Nullable Cache<Pair<String, String>, SegmentErrorInfo> errorCache,
- BooleanSupplier isServerReadyToServeQueries, boolean enableAsyncSegmentRefresh,
- ServerReloadJobStatusCache reloadJobStatusCache) {
+ BooleanSupplier isServerReadyToServeQueries, BooleanSupplier isIngestionPausedDueToStartUp,
Review Comment:
I'd model it to be more general: `isServerReadyToConsumeData`. It also aligns with `_isTableReadyToConsumeData`.
Put it before `isServerReadyToServeQueries` because it is a step before serving queries
##########
pinot-core/src/main/java/org/apache/pinot/core/data/manager/realtime/RealtimeTableDataManager.java:
##########
@@ -138,6 +138,7 @@ public class RealtimeTableDataManager extends BaseTableDataManager {
protected Cache<String, StreamMetadataProvider> _streamMetadataProviderCache;
private final BooleanSupplier _isServerReadyToServeQueries;
+ private final BooleanSupplier _isIngestionPausedDueToStartUp;
Review Comment:
(minor) Put it above ready to serve queries to follow the logical sequence. Same for other places
--
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: commits-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org