You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@pinot.apache.org by "KKcorps (via GitHub)" <gi...@apache.org> on 2023/07/14 10:57:22 UTC
[GitHub] [pinot] KKcorps opened a new pull request, #11110: Add stat to track number of segments that have valid doc id snapshots
KKcorps opened a new pull request, #11110:
URL: https://github.com/apache/pinot/pull/11110
* Need to track if valid doc id snapshot is working fine or not
* Can be tracked `upsertValidDocIdSnapshotCount` in your reporting tool.
--
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
[GitHub] [pinot] navina commented on pull request #11110: Add stat to track number of segments that have valid doc id snapshots
Posted by "navina (via GitHub)" <gi...@apache.org>.
navina commented on PR #11110:
URL: https://github.com/apache/pinot/pull/11110#issuecomment-1644415162
> > Changes look good. Trying to understand the motivation behind this metric.
>
> Basically we now create valid doc id snapshot on each segment commit. If these snapshots are not present, it affects the restart times for upsert tables by a lot. The metric can help us determine if we are good to rollout the servers or not.
Would be great if you can include this in the metric description!
--
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
[GitHub] [pinot] navina commented on a diff in pull request #11110: Add stat to track number of segments that have valid doc id snapshots
Posted by "navina (via GitHub)" <gi...@apache.org>.
navina commented on code in PR #11110:
URL: https://github.com/apache/pinot/pull/11110#discussion_r1264226124
##########
pinot-common/src/main/java/org/apache/pinot/common/metrics/ServerGauge.java:
##########
@@ -48,7 +48,8 @@ public enum ServerGauge implements AbstractMetrics.Gauge {
JVM_HEAP_USED_BYTES("bytes", true),
// Ingestion delay metrics
REALTIME_INGESTION_DELAY_MS("milliseconds", false),
- END_TO_END_REALTIME_INGESTION_DELAY_MS("milliseconds", false);
+ END_TO_END_REALTIME_INGESTION_DELAY_MS("milliseconds", false),
+ UPSERT_VALID_DOC_ID_SNAPSHOT_COUNT("upsertValidDocIdSnapshotCount", false);
Review Comment:
can you add a description along with this metric enum?
See example [here](https://github.com/apache/pinot/blob/master/pinot-common/src/main/java/org/apache/pinot/common/metrics/ServerTimer.java#L29)
--
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
[GitHub] [pinot] KKcorps commented on pull request #11110: Add stat to track number of segments that have valid doc id snapshots
Posted by "KKcorps (via GitHub)" <gi...@apache.org>.
KKcorps commented on PR #11110:
URL: https://github.com/apache/pinot/pull/11110#issuecomment-1643502721
> Changes look good. Trying to understand the motivation behind this metric.
Basically we now create valid doc id snapshot on each segment commit. If these snapshots are not present, it affects the restart times for upsert tables by a lot. The metric can help us determine if we are good to rollout the servers or not.
--
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
[GitHub] [pinot] codecov-commenter commented on pull request #11110: Add stat to track number of segments that have valid doc id snapshots
Posted by "codecov-commenter (via GitHub)" <gi...@apache.org>.
codecov-commenter commented on PR #11110:
URL: https://github.com/apache/pinot/pull/11110#issuecomment-1635731992
## [Codecov](https://app.codecov.io/gh/apache/pinot/pull/11110?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) Report
> Merging [#11110](https://app.codecov.io/gh/apache/pinot/pull/11110?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) (c6012b9) into [master](https://app.codecov.io/gh/apache/pinot/commit/35d7a8faa5e969602a637a68f158c51fdc594c38?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) (35d7a8f) will **increase** coverage by `0.00%`.
> The diff coverage is `0.00%`.
```diff
@@ Coverage Diff @@
## master #11110 +/- ##
=========================================
Coverage 0.11% 0.11%
=========================================
Files 2203 2148 -55
Lines 118165 115644 -2521
Branches 17879 17573 -306
=========================================
Hits 137 137
+ Misses 118008 115487 -2521
Partials 20 20
```
| Flag | Coverage Δ | |
|---|---|---|
| integration1temurin11 | `?` | |
| integration1temurin17 | `?` | |
| integration1temurin20 | `?` | |
| integration2temurin11 | `?` | |
| integration2temurin17 | `?` | |
| integration2temurin20 | `?` | |
| unittests1temurin17 | `?` | |
| unittests1temurin20 | `?` | |
| unittests2temurin11 | `?` | |
| unittests2temurin17 | `?` | |
| unittests2temurin20 | `0.11% <0.00%> (ø)` | |
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.
| [Impacted Files](https://app.codecov.io/gh/apache/pinot/pull/11110?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | Coverage Δ | |
|---|---|---|
| [...a/org/apache/pinot/common/metrics/ServerGauge.java](https://app.codecov.io/gh/apache/pinot/pull/11110?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-cGlub3QtY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9jb21tb24vbWV0cmljcy9TZXJ2ZXJHYXVnZS5qYXZh) | `0.00% <0.00%> (ø)` | |
| [...cal/upsert/BasePartitionUpsertMetadataManager.java](https://app.codecov.io/gh/apache/pinot/pull/11110?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-cGlub3Qtc2VnbWVudC1sb2NhbC9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3Qvc2VnbWVudC9sb2NhbC91cHNlcnQvQmFzZVBhcnRpdGlvblVwc2VydE1ldGFkYXRhTWFuYWdlci5qYXZh) | `0.00% <0.00%> (ø)` | |
... and [57 files with indirect coverage changes](https://app.codecov.io/gh/apache/pinot/pull/11110/indirect-changes?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)
:mega: We’re building smart automated test selection to slash your CI/CD build times. [Learn more](https://about.codecov.io/iterative-testing/?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
[GitHub] [pinot] navina commented on a diff in pull request #11110: Add stat to track number of segments that have valid doc id snapshots
Posted by "navina (via GitHub)" <gi...@apache.org>.
navina commented on code in PR #11110:
URL: https://github.com/apache/pinot/pull/11110#discussion_r1264225009
##########
pinot-segment-local/src/main/java/org/apache/pinot/segment/local/upsert/BasePartitionUpsertMetadataManager.java:
##########
@@ -540,6 +540,8 @@ protected void doTakeSnapshot() {
}
}
+ _serverMetrics.setValueOfTableGauge(_tableNameWithType, ServerGauge.UPSERT_VALID_DOC_ID_SNAPSHOT_COUNT,
Review Comment:
Dumb question: how is this metric going to be used? like what does it tell us?
Why is this metric defined at table level rather than a partition level ?
--
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
[GitHub] [pinot] navina commented on a diff in pull request #11110: Add stat to track number of segments that have valid doc id snapshots
Posted by "navina (via GitHub)" <gi...@apache.org>.
navina commented on code in PR #11110:
URL: https://github.com/apache/pinot/pull/11110#discussion_r1274113825
##########
pinot-common/src/main/java/org/apache/pinot/common/metrics/ServerGauge.java:
##########
@@ -48,7 +48,8 @@ public enum ServerGauge implements AbstractMetrics.Gauge {
JVM_HEAP_USED_BYTES("bytes", true),
// Ingestion delay metrics
REALTIME_INGESTION_DELAY_MS("milliseconds", false),
- END_TO_END_REALTIME_INGESTION_DELAY_MS("milliseconds", false);
+ END_TO_END_REALTIME_INGESTION_DELAY_MS("milliseconds", false),
+ UPSERT_VALID_DOC_ID_SNAPSHOT_COUNT("upsertValidDocIdSnapshotCount", false);
Review Comment:
@KKcorps is there a reason why you are not adding it as a enum field and simply a java comment?
--
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
[GitHub] [pinot] Jackie-Jiang commented on a diff in pull request #11110: Add stat to track number of segments that have valid doc id snapshots
Posted by "Jackie-Jiang (via GitHub)" <gi...@apache.org>.
Jackie-Jiang commented on code in PR #11110:
URL: https://github.com/apache/pinot/pull/11110#discussion_r1268831445
##########
pinot-segment-local/src/main/java/org/apache/pinot/segment/local/upsert/BasePartitionUpsertMetadataManager.java:
##########
@@ -540,6 +540,8 @@ protected void doTakeSnapshot() {
}
}
+ _serverMetrics.setValueOfTableGauge(_tableNameWithType, ServerGauge.UPSERT_VALID_DOC_ID_SNAPSHOT_COUNT,
Review Comment:
This needs to be a per partition gauge
--
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
[GitHub] [pinot] KKcorps commented on pull request #11110: Add stat to track number of segments that have valid doc id snapshots
Posted by "KKcorps (via GitHub)" <gi...@apache.org>.
KKcorps commented on PR #11110:
URL: https://github.com/apache/pinot/pull/11110#issuecomment-1649553354
> > > Changes look good. Trying to understand the motivation behind this metric.
> >
> >
> > Basically we now create valid doc id snapshot on each segment commit. If these snapshots are not present, it affects the restart times for upsert tables by a lot. The metric can help us determine if we are good to rollout the servers or not.
>
> Would be great if you can include this in the metric description!
already added as comment
--
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
[GitHub] [pinot] Jackie-Jiang merged pull request #11110: Add stat to track number of segments that have valid doc id snapshots
Posted by "Jackie-Jiang (via GitHub)" <gi...@apache.org>.
Jackie-Jiang merged PR #11110:
URL: https://github.com/apache/pinot/pull/11110
--
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