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/18 18:24:10 UTC
[GitHub] [pinot] KKcorps opened a new pull request, #11129: Bug fix: TableUpsertMetadataManager is null
KKcorps opened a new pull request, #11129:
URL: https://github.com/apache/pinot/pull/11129
* _tableUpsertMetadataManager variable is never initialised because we keep on waiting for `init`
* this leads to NullPointerException when trying to preload the segments
```
2023/07/18 10:20:13.526 INFO [ImmutableSegmentLoader] [segment-preload-thread-7] Successfully loaded segment: upsertMeetupRsvp_1678410006394_1678499996394_10 with SegmentDirectory
2023/07/18 10:20:13.526 INFO [upsertMeetupRsvpLarge_REALTIME-RealtimeTableDataManager] [segment-preload-thread-7] Adding immutable segment: upsertMeetupRsvp_1678410006394_1678499996394_10 to upsert-enabled table: upsertMeetupRsvpLarge_REALTIME
2023/07/18 10:20:13.526 ERROR [BaseTableDataManager] [segment-preload-thread-7] Failed to load existing segment: upsertMeetupRsvp_1678410006394_1678499996394_10 of table: upsertMeetupRsvpLarge_REALTIME with crc: 2719970321 on tier: default
java.lang.NullPointerException: null
at org.apache.pinot.core.data.manager.realtime.RealtimeTableDataManager.handleUpsert(RealtimeTableDataManager.java:534) ~[pinot-all-0.13.0-SNAPSHOT-jar-with-dependencies.jar:0.13.0-SNAPSHOT-47ec950074e6f5e6ce06671e376d828880b587d2]
at org.apache.pinot.core.data.manager.realtime.RealtimeTableDataManager.addSegment(RealtimeTableDataManager.java:499) ~[pinot-all-0.13.0-SNAPSHOT-jar-with-dependencies.jar:0.13.0-SNAPSHOT-47ec950074e6f5e6ce06671e376d828880b587d2]
at org.apache.pinot.core.data.manager.BaseTableDataManager.tryLoadExistingSegment(BaseTableDataManager.java:816) ~[pinot-all-0.13.0-SNAPSHOT-jar-with-dependencies.jar:0.13.0-SNAPSHOT-47ec950074e6f5e6ce06671e376d828880b587d2]
at org.apache.pinot.segment.local.upsert.BaseTableUpsertMetadataManager.preloadSegmentWithSnapshot(BaseTableUpsertMetadataManager.java:213) ~[pinot-all-0.13.0-SNAPSHOT-jar-with-dependencies.jar:0.13.0-SNAPSHOT-47ec950074e6f5e6ce06671e376d828880b587d2]
at org.apache.pinot.segment.local.upsert.BaseTableUpsertMetadataManager.preloadSegment(BaseTableUpsertMetadataManager.java:201) ~[pinot-all-0.13.0-SNAPSHOT-jar-with-dependencies.jar:0.13.0-SNAPSHOT-47ec950074e6f5e6ce06671e376d828880b587d2]
at org.apache.pinot.segment.local.upsert.BaseTableUpsertMetadataManager.lambda$preloadSegments$0(BaseTableUpsertMetadataManager.java:157) ~[pinot-all-0.13.0-SNAPSHOT-jar-with-dependencies.jar:0.13.0-SNAPSHOT-47ec950074e6f5e6ce06671e376d828880b587d2]
at java.util.concurrent.Executors$RunnableAdapter.call(Executors.java:515) [?:?]
at java.util.concurrent.FutureTask.run(FutureTask.java:264) [?:?]
at java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1128) [?:?]
at java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:628) [?:?]
at java.lang.Thread.run(Thread.java:829) [?:?]
2023/07/18 10:20:13.528 INFO [BaseTableUpsertMetadataManager] [segment-preload-thread-7] Preloaded segment: upsertMeetupRsvp_1678410006394_1678499996394_10 from table: upsertMeetupRsvpLarge_REALTIME
```
* solution is to seperate out the create and init steps.
--
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] klsince commented on a diff in pull request #11129: Bug fix: TableUpsertMetadataManager is null
Posted by "klsince (via GitHub)" <gi...@apache.org>.
klsince commented on code in PR #11129:
URL: https://github.com/apache/pinot/pull/11129#discussion_r1267178494
##########
pinot-core/src/main/java/org/apache/pinot/core/data/manager/realtime/RealtimeTableDataManager.java:
##########
@@ -210,8 +210,9 @@ protected void doInit() {
// this status with a boolean, instead of relying on if _tableUpsertMetadataManager is null or not.
_isUpsertEnabled = true;
_tableUpsertMetadataManager =
- TableUpsertMetadataManagerFactory.create(tableConfig, schema, this, _serverMetrics, _helixManager,
- _segmentPreloadExecutor);
+ TableUpsertMetadataManagerFactory.create(tableConfig);
Review Comment:
good catch. I think you can also remove `_isUpsertEnabled` bool flag and the comments about it on L209-L211 above. That bool flag was introduced to allow the blocking create() method.
--
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 #11129: Bug fix: TableUpsertMetadataManager is null
Posted by "Jackie-Jiang (via GitHub)" <gi...@apache.org>.
Jackie-Jiang merged PR #11129:
URL: https://github.com/apache/pinot/pull/11129
--
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 #11129: Bug fix: TableUpsertMetadataManager is null
Posted by "codecov-commenter (via GitHub)" <gi...@apache.org>.
codecov-commenter commented on PR #11129:
URL: https://github.com/apache/pinot/pull/11129#issuecomment-1640834002
## [Codecov](https://app.codecov.io/gh/apache/pinot/pull/11129?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) Report
> Merging [#11129](https://app.codecov.io/gh/apache/pinot/pull/11129?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) (c7c4158) into [master](https://app.codecov.io/gh/apache/pinot/commit/abc749774485405a75c1ec06dc6b75a1b89e68ce?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) (abc7497) will **decrease** coverage by `0.01%`.
> The diff coverage is `0.00%`.
```diff
@@ Coverage Diff @@
## master #11129 +/- ##
==========================================
- Coverage 0.11% 0.11% -0.01%
==========================================
Files 2203 2203
Lines 118196 118219 +23
Branches 17887 17891 +4
==========================================
Hits 137 137
- Misses 118039 118062 +23
Partials 20 20
```
| Flag | Coverage Δ | |
|---|---|---|
| integration1temurin11 | `?` | |
| integration1temurin17 | `0.00% <0.00%> (ø)` | |
| integration2temurin11 | `?` | |
| integration2temurin17 | `?` | |
| integration2temurin20 | `?` | |
| unittests1temurin11 | `?` | |
| unittests1temurin17 | `0.00% <0.00%> (ø)` | |
| unittests1temurin20 | `0.00% <0.00%> (ø)` | |
| unittests2temurin11 | `0.11% <0.00%> (-0.01%)` | :arrow_down: |
| unittests2temurin17 | `?` | |
| unittests2temurin20 | `0.11% <0.00%> (-0.01%)` | :arrow_down: |
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/11129?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | Coverage Δ | |
|---|---|---|
| [...ata/manager/realtime/RealtimeTableDataManager.java](https://app.codecov.io/gh/apache/pinot/pull/11129?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9kYXRhL21hbmFnZXIvcmVhbHRpbWUvUmVhbHRpbWVUYWJsZURhdGFNYW5hZ2VyLmphdmE=) | `0.00% <0.00%> (ø)` | |
| [...ocal/upsert/TableUpsertMetadataManagerFactory.java](https://app.codecov.io/gh/apache/pinot/pull/11129?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-cGlub3Qtc2VnbWVudC1sb2NhbC9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3Qvc2VnbWVudC9sb2NhbC91cHNlcnQvVGFibGVVcHNlcnRNZXRhZGF0YU1hbmFnZXJGYWN0b3J5LmphdmE=) | `0.00% <0.00%> (ø)` | |
... and [1 file with indirect coverage changes](https://app.codecov.io/gh/apache/pinot/pull/11129/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