You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@pinot.apache.org by "deepthi912 (via GitHub)" <gi...@apache.org> on 2026/10/08 23:58:01 UTC
[PR] Default pinot.server.consuming.segment.consistency.mode to PROTECTED [pinot]
deepthi912 opened a new pull request, #19010:
URL: https://github.com/apache/pinot/pull/19010
<!-- pinot-pr-flow:start -->
### PR flow
Changed default consuming segment consistency mode from RESTRICTED to PROTECTED, enabling force\-commit/reload with metadata reconciliation\.
```mermaid
flowchart TD
N0["Default mode#58; PROTECTED #40;F3#41;"]:::stModified
classDef stAdded fill:#dafbe1,stroke:#1a7f37,color:#1f2328,stroke-width:2px
classDef stModified fill:#fff8c5,stroke:#9a6700,color:#1f2328,stroke-width:2px
classDef stRemoved fill:#ffebe9,stroke:#cf222e,color:#1f2328,stroke-width:2px
classDef stUnchanged fill:#f6f8fa,stroke:#656d76,color:#1f2328,stroke-width:1px
```
AI-generated · Green: added · Yellow: modified · Red: removed · Gray: existing
<details>
<summary>Diff evidence</summary>
- F3: pinot\-spi/src/main/java/org/apache/pinot/spi/utils/ConsumingSegmentConsistencyModeListener\.java — [before](https://github.com/apache/pinot/blob/a979a6a0d8e173e887a68dd294e8013dc16ca43d/pinot-spi/src/main/java/org/apache/pinot/spi/utils/ConsumingSegmentConsistencyModeListener.java) · [after](https://github.com/apache/pinot/blob/637f900f054d85e894178b203c599bc8906fbb04/pinot-spi/src/main/java/org/apache/pinot/spi/utils/ConsumingSegmentConsistencyModeListener.java)
</details>
- [ ] Regenerate PR flow
<!-- pinot-pr-flow:meta eyJiYXNlX3NoYSI6ImE5NzlhNmEwZDhlMTczZTg4N2E2OGRkMjk0ZTgwMTNkYzE2Y2E0M2QiLCJjb250ZXh0X2hhc2giOiJmZjJmNjIzMzc0YTA0ZGRhYjBkMDQxZGQzYWEzNmVhYjg2NDU4ZTAyYmI4NWU2NGUxZWI0YWIwZDI3ZmNhMTI1IiwiZ2VuZXJhdGVkX2F0IjoxNzg5NTM4OTAyLCJoZWFkX3NoYSI6IjYzN2Y5MDBmMDU0ZDg1ZTg5NDE3OGIyMDNjNTk5YmM4OTA2ZmJiMDQiLCJtb2RlbCI6Im52aWRpYS9uZW1vdHJvbi0zLXN1cGVyLTEyMGItYTEyYjpmcmVlIiwib2JzZXJ2ZWRfYmFzZV9zaGEiOiJhOTc5YTZhMGQ4ZTE3M2U4ODdhNjhkZDI5NGU4MDEzZGMxNmNhNDNkIiwicG9saWN5IjoicGlub3QtcHItZmxvdy12MSJ9 -->
<!-- pinot-pr-flow:signature 54ed4c6650ab48f6fcaaa2ecc6d94f71d876288868a365bc85a8fe2a4ead9da6 -->
<!-- pinot-pr-flow:end -->
## Summary
`pinot.server.consuming.segment.consistency.mode` was introduced in #17503 with three modes:
- `RESTRICTED` (current default) — blocks force-commit/reload during the risky consuming-segment transition window for partial-upsert and `dropOutOfOrderRecord`/`outOfOrderRecordColumn` tables. It only *prevents* the risky operation from running; it doesn't resolve anything.
- `PROTECTED` — allows force-commit/reload, and reconciles the upsert metadata back to a consistent state if a discrepancy is detected during segment commit or removal.
- `UNSAFE` — allows force-commit/reload with no reconciliation.
This PR proposes flipping the default from `RESTRICTED` to `PROTECTED`, since `RESTRICTED` alone doesn't get an affected table to a correct state — it just blocks operational flows (reload, force-commit, Segment Refresh Task, index changes) on partial-upsert / `dropOutOfOrderRecord` tables indefinitely, until an operator manually opts into `PROTECTED`. Defaulting to `PROTECTED` means these operations work out of the box, with the metadata-reconciliation safety net already in place.
--
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] Default pinot.server.consuming.segment.consistency.mode to PROTECTED [pinot]
Posted by "deepthi912 (via GitHub)" <gi...@apache.org>.
deepthi912 commented on PR #19010:
URL: https://github.com/apache/pinot/pull/19010#issuecomment-5689432648
### CI failure analysis — `Pinot Integration Test Set 2 (temurin-25)` lane-a
The Set 2 lane-a failure is a Temurin 25.0.4.1+1 C2 JIT miscompilation, not a test regression from this PR. Sharing evidence so we can decide how to unblock.
**Crash signature (identical across 5 consecutive CI runs on this branch):**
```
# A fatal error has been detected by the Java Runtime Environment:
# SIGSEGV (0xb) at pc=0x00007f..., pid=..., tid=...
# JRE version: OpenJDK Runtime Environment Temurin-25.0.4.1+1 (25.0.4.1+1)
# Java VM: OpenJDK 64-Bit Server VM Temurin-25.0.4.1+1
# Problematic frame:
# J <id> c2 jdk.internal.misc.Unsafe.getIntUnaligned(Ljava/lang/Object;JZ)I java.base@25.0.4.1
```
Only the compilation ID (8023, 8750, 7858, 8172, ...) and PC address change between runs; frame and JDK are identical. The problematic frame is a C2-JIT-compiled JVM intrinsic inside `java.base` — a JDK codegen bug, not Pinot code.
**Why this PR trips it and master doesn't**
This PR flips the default `pinot.server.consuming.segment.consistency.mode` from `RESTRICTED` to `PROTECTED`. That default change alters the runtime path of two tests earlier in the same lane-a fork (`PauselessDedupRealtimeIngestionSegmentCommitFailureTest`, `PauselessRealtimeIngestionIntegrationTest`), which shifts C2's profile-guided inlining and codegen decisions enough to push `Unsafe.getIntUnaligned` into the buggy code path by the time `PartialUpsertTableRebalanceIntegrationTest` runs (~18 min into the fork, compilation ID ~7000-8500). Master's fork profile stays outside the buggy path.
**Local reproduction attempts**
| Setup | Result |
|---|---|
| macOS aarch64, Temurin 25.0.3+9, single test class, fresh fork | Passes in 37 s |
| macOS aarch64, Temurin **25.0.4.1+1** (identical to CI), single test class, fresh fork | Passes in 36 s |
| macOS aarch64, Temurin **25.0.4.1+1**, **7-test lane-a sequence** in single reused fork (`-DreuseForks=true -DforkCount=1`) | **Passes in 6:03 min** |
The last configuration matches CI's JDK version, test sequence, and fork-reuse strategy — but the C2 bug does not reproduce on aarch64 because C2 emits completely different machine code per architecture. The bug appears specific to Temurin 25.0.4.1+1 x86-64 C2 output.
**Options to unblock — deferring to maintainer preference**
1. **Pin Temurin patch level** to 25.0.3.x (or later 25.0.4.x once a fix ships) in `.github/workflows/pinot_integration_tests.yml` — currently `java: [ 25 ]` resolves to whatever is latest at run time.
2. **Add `-XX:CompileCommand=exclude,jdk/internal/misc/Unsafe.getIntUnaligned`** to `pinot.integration.test.jvm.args` in `pinot-integration-tests/pom.xml` — disables C2 for that one intrinsic (test-only, ~1% perf hit on byte[]/String hashing), avoids the crash. Small, targeted, reversible.
3. **Waive the required check on this PR and merge** — the semantic change is a one-line default flip; it cannot itself cause a native SIGSEGV.
4. **Wait for Adoptium to ship a fix** — happy to file the bug at https://github.com/adoptium/adoptium-support/issues with a full `hs_err_pid*.log` if the workflow can be tweaked to upload it as an artifact on failure (currently not uploaded, so the full crashing-thread stack is not accessible).
Happy to open a separate PR for whichever workaround the maintainers prefer. Not planning to keep re-running CI on this PR — 5 for 5 with identical signature has established the pattern.
--
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] Default pinot.server.consuming.segment.consistency.mode to PROTECTED [pinot]
Posted by "deepthi912 (via GitHub)" <gi...@apache.org>.
deepthi912 commented on code in PR #19010:
URL: https://github.com/apache/pinot/pull/19010#discussion_r4021512124
##########
pinot-segment-local/src/main/java/org/apache/pinot/segment/local/upsert/ConcurrentMapPartitionUpsertMetadataManager.java:
##########
@@ -240,6 +240,10 @@ protected void revertAndRemoveSegment(IndexSegment segment,
}
// Revert to previous segment location
IndexSegment prevSegment = prevLocation.getSegment();
+ // Skip if prevSegment has been offloaded; its data buffers are unmapped.
Review Comment:
Noticed SIGSEGV when running `RocksDBPartialUpsertTableRebalanceIntegrationTest`, felt like this is the only pplace we are accessing the prev segment location without checking. Locally I don't see any issues. So pushed it to check against CI.
--
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] Default pinot.server.consuming.segment.consistency.mode to PROTECTED [pinot]
Posted by "codecov-commenter (via GitHub)" <gi...@apache.org>.
codecov-commenter commented on PR #19010:
URL: https://github.com/apache/pinot/pull/19010#issuecomment-5010004509
## [Codecov](https://app.codecov.io/gh/apache/pinot/pull/19010?dropdown=coverage&src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) Report
:white_check_mark: All modified and coverable lines are covered by tests.
:white_check_mark: Project coverage is 37.69%. Comparing base ([`41ee76b`](https://app.codecov.io/gh/apache/pinot/commit/41ee76b7ba66a5e7d200d99591544a58689eeb7e?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)) to head ([`d7d6c14`](https://app.codecov.io/gh/apache/pinot/commit/d7d6c1463d47d1d5bd5934abc8b6c79921de98db?dropdown=coverage&el=desc&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 (41ee76b) and HEAD (d7d6c14). Click for more details.
>
> <details><summary>HEAD has 16 uploads less than BASE</summary>
>
>| Flag | BASE (41ee76b) | HEAD (d7d6c14) |
>|------|------|------|
>|java-21|5|1|
>|unittests1|1|0|
>|unittests|2|1|
>|temurin|5|1|
>|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 #19010 +/- ##
=============================================
- Coverage 65.25% 37.69% -27.56%
+ Complexity 1405 1398 -7
=============================================
Files 3420 3420
Lines 215022 215022
Branches 34021 34021
=============================================
- Hits 140312 81054 -59258
- Misses 63392 126557 +63165
+ Partials 11318 7411 -3907
```
| [Flag](https://app.codecov.io/gh/apache/pinot/pull/19010/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/19010/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/19010/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/19010/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/19010/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/19010/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `37.69% <100.00%> (-27.56%)` | :arrow_down: |
| [temurin](https://app.codecov.io/gh/apache/pinot/pull/19010/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `37.69% <100.00%> (-27.56%)` | :arrow_down: |
| [unittests](https://app.codecov.io/gh/apache/pinot/pull/19010/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `37.69% <100.00%> (-27.56%)` | :arrow_down: |
| [unittests1](https://app.codecov.io/gh/apache/pinot/pull/19010/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `?` | |
| [unittests2](https://app.codecov.io/gh/apache/pinot/pull/19010/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `37.69% <100.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.
</details>
[:umbrella: View full report in Codecov by Harness](https://app.codecov.io/gh/apache/pinot/pull/19010?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] Default pinot.server.consuming.segment.consistency.mode to PROTECTED [pinot]
Posted by "deepthi912 (via GitHub)" <gi...@apache.org>.
deepthi912 commented on code in PR #19010:
URL: https://github.com/apache/pinot/pull/19010#discussion_r4023375551
##########
pinot-segment-local/src/main/java/org/apache/pinot/segment/local/upsert/ConcurrentMapPartitionUpsertMetadataManager.java:
##########
@@ -240,6 +240,10 @@ protected void revertAndRemoveSegment(IndexSegment segment,
}
// Revert to previous segment location
IndexSegment prevSegment = prevLocation.getSegment();
+ // Skip if prevSegment has been offloaded; its data buffers are unmapped.
Review Comment:
Removed the check here and made sure the segments are removed from the `_previousKeyToRecordLocationMap` when prev segment locations are removed and untracked.
--
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] Default pinot.server.consuming.segment.consistency.mode to PROTECTED [pinot]
Posted by "deepthi912 (via GitHub)" <gi...@apache.org>.
deepthi912 commented on code in PR #19010:
URL: https://github.com/apache/pinot/pull/19010#discussion_r4030893676
##########
pinot-segment-local/src/main/java/org/apache/pinot/segment/local/upsert/ConcurrentMapPartitionUpsertMetadataManager.java:
##########
@@ -240,6 +240,10 @@ protected void revertAndRemoveSegment(IndexSegment segment,
}
// Revert to previous segment location
IndexSegment prevSegment = prevLocation.getSegment();
+ // Skip if prevSegment has been offloaded; its data buffers are unmapped.
Review Comment:
I feel This is not as well thread safe solution.
We need a better way to fix this ordering problem during offload.
--
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] Default pinot.server.consuming.segment.consistency.mode to PROTECTED [pinot]
Posted by "deepthi912 (via GitHub)" <gi...@apache.org>.
deepthi912 commented on code in PR #19010:
URL: https://github.com/apache/pinot/pull/19010#discussion_r4030893676
##########
pinot-segment-local/src/main/java/org/apache/pinot/segment/local/upsert/ConcurrentMapPartitionUpsertMetadataManager.java:
##########
@@ -240,6 +240,10 @@ protected void revertAndRemoveSegment(IndexSegment segment,
}
// Revert to previous segment location
IndexSegment prevSegment = prevLocation.getSegment();
+ // Skip if prevSegment has been offloaded; its data buffers are unmapped.
Review Comment:
This is not as well thread safe operation. I think we need a better way to fix this ordering problem during offload.
##########
pinot-segment-local/src/main/java/org/apache/pinot/segment/local/upsert/ConcurrentMapPartitionUpsertMetadataManager.java:
##########
@@ -240,6 +240,10 @@ protected void revertAndRemoveSegment(IndexSegment segment,
}
// Revert to previous segment location
IndexSegment prevSegment = prevLocation.getSegment();
+ // Skip if prevSegment has been offloaded; its data buffers are unmapped.
Review Comment:
I feel This is not as well thread safe operation. I think we need a better way to fix this ordering problem during offload.
--
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] Default pinot.server.consuming.segment.consistency.mode to PROTECTED [pinot]
Posted by "deepthi912 (via GitHub)" <gi...@apache.org>.
deepthi912 commented on code in PR #19010:
URL: https://github.com/apache/pinot/pull/19010#discussion_r4023375551
##########
pinot-segment-local/src/main/java/org/apache/pinot/segment/local/upsert/ConcurrentMapPartitionUpsertMetadataManager.java:
##########
@@ -240,6 +240,10 @@ protected void revertAndRemoveSegment(IndexSegment segment,
}
// Revert to previous segment location
IndexSegment prevSegment = prevLocation.getSegment();
+ // Skip if prevSegment has been offloaded; its data buffers are unmapped.
Review Comment:
Removed the check here and made sure the segments are removed from the `_previousKeyToRecordLocationMap` when prev segment locations are removed and untracked which can happen during rebalance or segment deletions
--
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] Default pinot.server.consuming.segment.consistency.mode to PROTECTED [pinot]
Posted by "deepthi912 (via GitHub)" <gi...@apache.org>.
deepthi912 commented on code in PR #19010:
URL: https://github.com/apache/pinot/pull/19010#discussion_r4023375551
##########
pinot-segment-local/src/main/java/org/apache/pinot/segment/local/upsert/ConcurrentMapPartitionUpsertMetadataManager.java:
##########
@@ -240,6 +240,10 @@ protected void revertAndRemoveSegment(IndexSegment segment,
}
// Revert to previous segment location
IndexSegment prevSegment = prevLocation.getSegment();
+ // Skip if prevSegment has been offloaded; its data buffers are unmapped.
Review Comment:
Removed the check here and made sure the segments are removed from the `_previousKeyToRecordLocationMap` when segments are removed and untracked.
--
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] Default pinot.server.consuming.segment.consistency.mode to PROTECTED [pinot]
Posted by "xiangfu0 (via GitHub)" <gi...@apache.org>.
xiangfu0 commented on code in PR #19010:
URL: https://github.com/apache/pinot/pull/19010#discussion_r3607768928
##########
pinot-spi/src/main/java/org/apache/pinot/spi/utils/ConsumingSegmentConsistencyModeListener.java:
##########
@@ -59,7 +60,7 @@ public enum Mode {
*/
UNSAFE(true);
- public static final Mode DEFAULT_CONSUMING_SEGMENT_CONSISTENCY_MODE = RESTRICTED;
+ public static final Mode DEFAULT_CONSUMING_SEGMENT_CONSISTENCY_MODE = PROTECTED;
Review Comment:
This changes the released cluster-wide default from `RESTRICTED` to `PROTECTED` in `pinot-spi`. Deployments that never set `pinot.server.consuming.segment.consistency.mode` would now allow force-commit/reload on partial-upsert or `dropOutOfOrderRecord` tables and run the metadata-revert path by default. Keep the Apache default `RESTRICTED`, or make `PROTECTED` an explicit cluster config/upgrade opt-in.
--
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] Default pinot.server.consuming.segment.consistency.mode to PROTECTED [pinot]
Posted by "Jackie-Jiang (via GitHub)" <gi...@apache.org>.
Jackie-Jiang commented on code in PR #19010:
URL: https://github.com/apache/pinot/pull/19010#discussion_r4021484945
##########
pinot-segment-local/src/main/java/org/apache/pinot/segment/local/upsert/ConcurrentMapPartitionUpsertMetadataManager.java:
##########
@@ -240,6 +240,10 @@ protected void revertAndRemoveSegment(IndexSegment segment,
}
// Revert to previous segment location
IndexSegment prevSegment = prevLocation.getSegment();
+ // Skip if prevSegment has been offloaded; its data buffers are unmapped.
Review Comment:
Is this a bug fix? I don't think a check here is thread sage. Is there race condition when the segment is offloaded after this?
--
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] Default pinot.server.consuming.segment.consistency.mode to PROTECTED [pinot]
Posted by "deepthi912 (via GitHub)" <gi...@apache.org>.
deepthi912 commented on code in PR #19010:
URL: https://github.com/apache/pinot/pull/19010#discussion_r4030893676
##########
pinot-segment-local/src/main/java/org/apache/pinot/segment/local/upsert/ConcurrentMapPartitionUpsertMetadataManager.java:
##########
@@ -240,6 +240,10 @@ protected void revertAndRemoveSegment(IndexSegment segment,
}
// Revert to previous segment location
IndexSegment prevSegment = prevLocation.getSegment();
+ // Skip if prevSegment has been offloaded; its data buffers are unmapped.
Review Comment:
I feel This is not as well thread safe solution. I think we need a better way to fix this ordering problem during offload.
--
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] Default pinot.server.consuming.segment.consistency.mode to PROTECTED [pinot]
Posted by "Jackie-Jiang (via GitHub)" <gi...@apache.org>.
Jackie-Jiang merged PR #19010:
URL: https://github.com/apache/pinot/pull/19010
--
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] Default pinot.server.consuming.segment.consistency.mode to PROTECTED [pinot]
Posted by "deepthi912 (via GitHub)" <gi...@apache.org>.
deepthi912 commented on code in PR #19010:
URL: https://github.com/apache/pinot/pull/19010#discussion_r4021512124
##########
pinot-segment-local/src/main/java/org/apache/pinot/segment/local/upsert/ConcurrentMapPartitionUpsertMetadataManager.java:
##########
@@ -240,6 +240,10 @@ protected void revertAndRemoveSegment(IndexSegment segment,
}
// Revert to previous segment location
IndexSegment prevSegment = prevLocation.getSegment();
+ // Skip if prevSegment has been offloaded; its data buffers are unmapped.
Review Comment:
Noticed SIGSEGV in multiple runs when running `RocksDBPartialUpsertTableRebalanceIntegrationTest`, felt like this is the only pplace we are accessing the prev segment location without checking. Locally I don't see any issues. So pushed it to check against CI.
--
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] Default pinot.server.consuming.segment.consistency.mode to PROTECTED [pinot]
Posted by "deepthi912 (via GitHub)" <gi...@apache.org>.
deepthi912 commented on PR #19010:
URL: https://github.com/apache/pinot/pull/19010#issuecomment-5706497600
Closing this as there is bigger bug to be fixed before making it on by default
--
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] Default pinot.server.consuming.segment.consistency.mode to PROTECTED [pinot]
Posted by "deepthi912 (via GitHub)" <gi...@apache.org>.
deepthi912 closed pull request #19010: Default pinot.server.consuming.segment.consistency.mode to PROTECTED
URL: https://github.com/apache/pinot/pull/19010
--
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] Default pinot.server.consuming.segment.consistency.mode to PROTECTED [pinot]
Posted by "xiangfu0 (via GitHub)" <gi...@apache.org>.
xiangfu0 commented on PR #19010:
URL: https://github.com/apache/pinot/pull/19010#issuecomment-6131340602
Documentation follow-up merged: https://github.com/pinot-contrib/pinot-docs/pull/1099. Updates the PROTECTED default, explicit RESTRICTED behavior, schema-evolution guidance, and parallel-consumption upgrade requirements.
--
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