You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@pinot.apache.org by "suddendust (via GitHub)" <gi...@apache.org> on 2023/11/14 17:06:45 UTC
[PR] Expose a Gauge for Ingestion Paused [pinot]
suddendust opened a new pull request, #12000:
URL: https://github.com/apache/pinot/pull/12000
We need to expose a metric to identify when a table is paused.
--
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] Expose a Gauge for Table Disabled and Consumption Paused [pinot]
Posted by "suddendust (via GitHub)" <gi...@apache.org>.
suddendust commented on code in PR #12000:
URL: https://github.com/apache/pinot/pull/12000#discussion_r1411228807
##########
pinot-controller/src/main/java/org/apache/pinot/controller/helix/core/realtime/PinotLLCRealtimeSegmentManager.java:
##########
@@ -871,8 +872,8 @@ public void ensureAllPartitionsConsuming(TableConfig tableConfig, StreamConfig s
boolean offsetsHaveToChange = offsetCriteria != null;
if (isTableEnabled && !isTablePaused) {
List<PartitionGroupConsumptionStatus> currentPartitionGroupConsumptionStatusList =
- offsetsHaveToChange
- ? Collections.emptyList() // offsets from metadata are not valid anymore; fetch for all partitions
+ offsetsHaveToChange ? Collections.emptyList()
Review Comment:
Reverted this change.
##########
pinot-controller/src/main/java/org/apache/pinot/controller/helix/core/realtime/PinotLLCRealtimeSegmentManager.java:
##########
@@ -1531,9 +1530,10 @@ public PauseStatus pauseConsumption(String tableNameWithType) {
IdealState updatedIdealState = updatePauseStatusInIdealState(tableNameWithType, true);
Set<String> consumingSegments = findConsumingSegments(updatedIdealState);
sendForceCommitMessageToServers(tableNameWithType, consumingSegments);
- return new PauseStatus(true, consumingSegments, consumingSegments.isEmpty() ? null : "Pause flag is set."
- + " Consuming segments are being committed."
- + " Use /pauseStatus endpoint in a few moments to check if all consuming segments have been committed.");
+ _controllerMetrics.setValueOfTableGauge(tableNameWithType, ControllerGauge.TABLE_CONSUMPTION_PAUSED, 1);
Review Comment:
Addressed.
--
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] Metrics for Table Disabled and Consumption Paused [pinot]
Posted by "walterddr (via GitHub)" <gi...@apache.org>.
walterddr commented on code in PR #12000:
URL: https://github.com/apache/pinot/pull/12000#discussion_r1419175778
##########
pinot-controller/src/main/java/org/apache/pinot/controller/helix/core/realtime/PinotLLCRealtimeSegmentManager.java:
##########
@@ -129,14 +129,14 @@
* TODO: migrate code in this class to other places for better readability
*/
public class PinotLLCRealtimeSegmentManager {
+
+ // simple field in Ideal State representing pause status for the table
+ public static final String IS_TABLE_PAUSED = "isTablePaused";
Review Comment:
what was this change for?
--
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] Expose a Gauge for Table Disabled and Consumption Paused [pinot]
Posted by "Jackie-Jiang (via GitHub)" <gi...@apache.org>.
Jackie-Jiang commented on code in PR #12000:
URL: https://github.com/apache/pinot/pull/12000#discussion_r1411324944
##########
pinot-common/src/main/java/org/apache/pinot/common/metrics/ControllerGauge.java:
##########
@@ -147,7 +147,11 @@ public enum ControllerGauge implements AbstractMetrics.Gauge {
FAILED_TO_COPY_SCHEMA_COUNT("failedToCopySchemaCount", true),
// Number of tables that we want to fix but failed to update table config
- FAILED_TO_UPDATE_TABLE_CONFIG_COUNT("failedToUpdateTableConfigCount", true);
+ FAILED_TO_UPDATE_TABLE_CONFIG_COUNT("failedToUpdateTableConfigCount", true),
+
+ TABLE_CONSUMPTION_PAUSED("tableConsumptionPaused", false),
+
+ TABLE_ENABLED("tableEnabled", false);
Review Comment:
Let's change it to `TABLE_DISABLED`. We don't really care how many table is enabled, but should be alerted if some tables are disabled
##########
pinot-controller/src/main/java/org/apache/pinot/controller/helix/core/periodictask/ControllerPeriodicTask.java:
##########
@@ -120,6 +126,7 @@ protected void processTables(List<String> tableNamesWithType, Properties periodi
ControllerMeter.PERIODIC_TASK_ERROR, 1L);
}
numTablesProcessed++;
+ exposeTableStates(tableNameWithType);
Review Comment:
This should be done within one specific periodic task, but not all of them. Take a look at `SegmentStatusChecker` where we already track disabled tables, but not emitting metrics as of now
--
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] Expose a Gauge for Ingestion Paused [pinot]
Posted by "suddendust (via GitHub)" <gi...@apache.org>.
suddendust commented on code in PR #12000:
URL: https://github.com/apache/pinot/pull/12000#discussion_r1410425979
##########
pinot-controller/src/main/java/org/apache/pinot/controller/helix/core/realtime/PinotLLCRealtimeSegmentManager.java:
##########
@@ -1531,9 +1530,10 @@ public PauseStatus pauseConsumption(String tableNameWithType) {
IdealState updatedIdealState = updatePauseStatusInIdealState(tableNameWithType, true);
Set<String> consumingSegments = findConsumingSegments(updatedIdealState);
sendForceCommitMessageToServers(tableNameWithType, consumingSegments);
- return new PauseStatus(true, consumingSegments, consumingSegments.isEmpty() ? null : "Pause flag is set."
- + " Consuming segments are being committed."
- + " Use /pauseStatus endpoint in a few moments to check if all consuming segments have been committed.");
+ _controllerMetrics.setValueOfTableGauge(tableNameWithType, ControllerGauge.TABLE_CONSUMPTION_PAUSED, 1);
Review Comment:
`ensureAllPartitionsConsuming` is also in the controller, so if it restarts, will it capture it?
--
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] Metrics for Table Disabled and Consumption Paused [pinot]
Posted by "walterddr (via GitHub)" <gi...@apache.org>.
walterddr commented on code in PR #12000:
URL: https://github.com/apache/pinot/pull/12000#discussion_r1419175778
##########
pinot-controller/src/main/java/org/apache/pinot/controller/helix/core/realtime/PinotLLCRealtimeSegmentManager.java:
##########
@@ -129,14 +129,14 @@
* TODO: migrate code in this class to other places for better readability
*/
public class PinotLLCRealtimeSegmentManager {
+
+ // simple field in Ideal State representing pause status for the table
+ public static final String IS_TABLE_PAUSED = "isTablePaused";
Review Comment:
what was this change for?
--
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] Expose a Gauge for Ingestion Paused [pinot]
Posted by "codecov-commenter (via GitHub)" <gi...@apache.org>.
codecov-commenter commented on PR #12000:
URL: https://github.com/apache/pinot/pull/12000#issuecomment-1810809297
## [Codecov](https://app.codecov.io/gh/apache/pinot/pull/12000?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) Report
Attention: `14 lines` in your changes are missing coverage. Please review.
> Comparison is base [(`18639b9`)](https://app.codecov.io/gh/apache/pinot/commit/18639b94438f87a1da4a8a36256a2a672e4cc8b4?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) 61.42% compared to head [(`7c8b447`)](https://app.codecov.io/gh/apache/pinot/pull/12000?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) 34.92%.
> Report is 36 commits behind head on master.
| [Files](https://app.codecov.io/gh/apache/pinot/pull/12000?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | Patch % | Lines |
|---|---|---|
| [.../core/realtime/PinotLLCRealtimeSegmentManager.java](https://app.codecov.io/gh/apache/pinot/pull/12000?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-cGlub3QtY29udHJvbGxlci9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29udHJvbGxlci9oZWxpeC9jb3JlL3JlYWx0aW1lL1Bpbm90TExDUmVhbHRpbWVTZWdtZW50TWFuYWdlci5qYXZh) | 0.00% | [12 Missing :warning: ](https://app.codecov.io/gh/apache/pinot/pull/12000?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
| [...g/apache/pinot/common/metrics/ControllerGauge.java](https://app.codecov.io/gh/apache/pinot/pull/12000?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-cGlub3QtY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9jb21tb24vbWV0cmljcy9Db250cm9sbGVyR2F1Z2UuamF2YQ==) | 0.00% | [2 Missing :warning: ](https://app.codecov.io/gh/apache/pinot/pull/12000?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
<details><summary>Additional details and impacted files</summary>
```diff
@@ Coverage Diff @@
## master #12000 +/- ##
=============================================
- Coverage 61.42% 34.92% -26.51%
+ Complexity 1147 943 -204
=============================================
Files 2378 2309 -69
Lines 128894 125491 -3403
Branches 19929 19448 -481
=============================================
- Hits 79177 43823 -35354
- Misses 44001 78552 +34551
+ Partials 5716 3116 -2600
```
| [Flag](https://app.codecov.io/gh/apache/pinot/pull/12000/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/12000/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/12000/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `0.00% <0.00%> (-0.01%)` | :arrow_down: |
| [integration1](https://app.codecov.io/gh/apache/pinot/pull/12000/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/12000/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `0.00% <0.00%> (ø)` | |
| [java-11](https://app.codecov.io/gh/apache/pinot/pull/12000/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/12000/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `34.92% <0.00%> (-26.39%)` | :arrow_down: |
| [skip-bytebuffers-false](https://app.codecov.io/gh/apache/pinot/pull/12000/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `34.90% <0.00%> (-26.51%)` | :arrow_down: |
| [skip-bytebuffers-true](https://app.codecov.io/gh/apache/pinot/pull/12000/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `34.90% <0.00%> (-26.37%)` | :arrow_down: |
| [temurin](https://app.codecov.io/gh/apache/pinot/pull/12000/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `34.92% <0.00%> (-26.51%)` | :arrow_down: |
| [unittests](https://app.codecov.io/gh/apache/pinot/pull/12000/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `46.78% <0.00%> (-14.64%)` | :arrow_down: |
| [unittests1](https://app.codecov.io/gh/apache/pinot/pull/12000/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `46.78% <0.00%> (+0.15%)` | :arrow_up: |
| [unittests2](https://app.codecov.io/gh/apache/pinot/pull/12000/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/12000?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).
--
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] Metrics for Table Disabled and Consumption Paused [pinot]
Posted by "walterddr (via GitHub)" <gi...@apache.org>.
walterddr merged PR #12000:
URL: https://github.com/apache/pinot/pull/12000
--
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] Expose a Gauge for Table Disabled and Consumption Paused [pinot]
Posted by "suddendust (via GitHub)" <gi...@apache.org>.
suddendust commented on code in PR #12000:
URL: https://github.com/apache/pinot/pull/12000#discussion_r1411684672
##########
pinot-controller/src/main/java/org/apache/pinot/controller/helix/core/periodictask/ControllerPeriodicTask.java:
##########
@@ -120,6 +126,7 @@ protected void processTables(List<String> tableNamesWithType, Properties periodi
ControllerMeter.PERIODIC_TASK_ERROR, 1L);
}
numTablesProcessed++;
+ exposeTableStates(tableNameWithType);
Review Comment:
Noted, moved to SegmentStatusChecker.
--
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] Metrics for Table Disabled and Consumption Paused [pinot]
Posted by "suddendust (via GitHub)" <gi...@apache.org>.
suddendust commented on code in PR #12000:
URL: https://github.com/apache/pinot/pull/12000#discussion_r1414507420
##########
pinot-controller/src/main/java/org/apache/pinot/controller/helix/SegmentStatusChecker.java:
##########
@@ -61,6 +61,7 @@
public class SegmentStatusChecker extends ControllerPeriodicTask<SegmentStatusChecker.Context> {
private static final Logger LOGGER = LoggerFactory.getLogger(SegmentStatusChecker.class);
private static final int MAX_OFFLINE_SEGMENTS_TO_LOG = 5;
+ private static final String TABLE_CONSUMPTION_PAUSED = "isTablePaused";
Review Comment:
Actually, this is the key that is set is the table idealstate when a table is paused. So will have to use it to for lookup.
--
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] Metrics for Table Disabled and Consumption Paused [pinot]
Posted by "Jackie-Jiang (via GitHub)" <gi...@apache.org>.
Jackie-Jiang commented on code in PR #12000:
URL: https://github.com/apache/pinot/pull/12000#discussion_r1414740282
##########
pinot-controller/src/main/java/org/apache/pinot/controller/helix/SegmentStatusChecker.java:
##########
@@ -369,5 +396,7 @@ public static final class Context {
private int _offlineTableCount;
private int _disabledTableCount;
private Set<String> _processedTables = new HashSet<>();
+ private Set<String> _disabledTables = new HashSet<>();
Review Comment:
Since we already track this set, no need to track `_disabledTableCount`
##########
pinot-controller/src/main/java/org/apache/pinot/controller/helix/SegmentStatusChecker.java:
##########
@@ -61,6 +61,7 @@
public class SegmentStatusChecker extends ControllerPeriodicTask<SegmentStatusChecker.Context> {
private static final Logger LOGGER = LoggerFactory.getLogger(SegmentStatusChecker.class);
private static final int MAX_OFFLINE_SEGMENTS_TO_LOG = 5;
+ private static final String IS_TABLE_CONSUMPTION_PAUSED = "isTablePaused";
Review Comment:
Let's reuse `PinotLLCRealtimeSegmentManager.IS_TABLE_PAUSED` (make it public)
--
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] Metrics for Table Disabled and Consumption Paused [pinot]
Posted by "walterddr (via GitHub)" <gi...@apache.org>.
walterddr commented on code in PR #12000:
URL: https://github.com/apache/pinot/pull/12000#discussion_r1419182229
##########
pinot-controller/src/main/java/org/apache/pinot/controller/helix/core/realtime/PinotLLCRealtimeSegmentManager.java:
##########
@@ -129,14 +129,14 @@
* TODO: migrate code in this class to other places for better readability
*/
public class PinotLLCRealtimeSegmentManager {
+
+ // simple field in Ideal State representing pause status for the table
+ public static final String IS_TABLE_PAUSED = "isTablePaused";
Review Comment:
instead of exposing a private static field. we can probably expose the isTablePause method as public static. but can be follow up
--
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] Expose a Gauge for Ingestion Paused [pinot]
Posted by "Jackie-Jiang (via GitHub)" <gi...@apache.org>.
Jackie-Jiang commented on code in PR #12000:
URL: https://github.com/apache/pinot/pull/12000#discussion_r1395006387
##########
pinot-controller/src/main/java/org/apache/pinot/controller/helix/core/realtime/PinotLLCRealtimeSegmentManager.java:
##########
@@ -1531,9 +1530,10 @@ public PauseStatus pauseConsumption(String tableNameWithType) {
IdealState updatedIdealState = updatePauseStatusInIdealState(tableNameWithType, true);
Set<String> consumingSegments = findConsumingSegments(updatedIdealState);
sendForceCommitMessageToServers(tableNameWithType, consumingSegments);
- return new PauseStatus(true, consumingSegments, consumingSegments.isEmpty() ? null : "Pause flag is set."
- + " Consuming segments are being committed."
- + " Use /pauseStatus endpoint in a few moments to check if all consuming segments have been committed.");
+ _controllerMetrics.setValueOfTableGauge(tableNameWithType, ControllerGauge.TABLE_CONSUMPTION_PAUSED, 1);
Review Comment:
This won't capture the paused table when controller is restarted. We need to set this gauge when a segment commits, or in `ensureAllPartitionsConsuming()`
##########
pinot-controller/src/main/java/org/apache/pinot/controller/helix/core/realtime/PinotLLCRealtimeSegmentManager.java:
##########
@@ -871,8 +872,8 @@ public void ensureAllPartitionsConsuming(TableConfig tableConfig, StreamConfig s
boolean offsetsHaveToChange = offsetCriteria != null;
if (isTableEnabled && !isTablePaused) {
List<PartitionGroupConsumptionStatus> currentPartitionGroupConsumptionStatusList =
- offsetsHaveToChange
- ? Collections.emptyList() // offsets from metadata are not valid anymore; fetch for all partitions
+ offsetsHaveToChange ? Collections.emptyList()
Review Comment:
(minor) This is not as readable. Suggest not reformatting unrelated code
--
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] Metrics for Table Disabled and Consumption Paused [pinot]
Posted by "zhtaoxiang (via GitHub)" <gi...@apache.org>.
zhtaoxiang commented on code in PR #12000:
URL: https://github.com/apache/pinot/pull/12000#discussion_r1414252595
##########
docker/images/pinot/etc/jmx_prometheus_javaagent/configs/controller.yml:
##########
@@ -204,6 +204,18 @@ rules:
cache: true
labels:
version: "$2"
+- pattern: "\"org.apache.pinot.common.metrics\"<type=\"ControllerMetrics\", name=\"pinot.controller.tableConsumptionPaused.([^\\.]*?)_(OFFLINE|REALTIME)\"><>(\\w+)"
+ name: "pinot_controller_tableConsumptionPaused_$3"
+ cache: true
+ labels:
+ tableName: "$1"
+ tableType: "$2"
Review Comment:
nit: I think table consumption only applies to realtime table?
##########
pinot-controller/src/main/java/org/apache/pinot/controller/helix/SegmentStatusChecker.java:
##########
@@ -61,6 +61,7 @@
public class SegmentStatusChecker extends ControllerPeriodicTask<SegmentStatusChecker.Context> {
private static final Logger LOGGER = LoggerFactory.getLogger(SegmentStatusChecker.class);
private static final int MAX_OFFLINE_SEGMENTS_TO_LOG = 5;
+ private static final String TABLE_CONSUMPTION_PAUSED = "isTablePaused";
Review Comment:
Can we use `isTableConsumptionPaused` to make it easy to read?
--
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] Metrics for Table Disabled and Consumption Paused [pinot]
Posted by "suddendust (via GitHub)" <gi...@apache.org>.
suddendust commented on code in PR #12000:
URL: https://github.com/apache/pinot/pull/12000#discussion_r1414503538
##########
docker/images/pinot/etc/jmx_prometheus_javaagent/configs/controller.yml:
##########
@@ -204,6 +204,18 @@ rules:
cache: true
labels:
version: "$2"
+- pattern: "\"org.apache.pinot.common.metrics\"<type=\"ControllerMetrics\", name=\"pinot.controller.tableConsumptionPaused.([^\\.]*?)_(OFFLINE|REALTIME)\"><>(\\w+)"
+ name: "pinot_controller_tableConsumptionPaused_$3"
+ cache: true
+ labels:
+ tableName: "$1"
+ tableType: "$2"
Review Comment:
Maybe in future we can add an ability to pause consumption in offline tables too? I think we can keep this label.
--
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