Group commit IdealState updates#13976
Conversation
pinot-common/src/main/java/org/apache/pinot/common/utils/helix/IdealStateGroupCommit.java
Outdated
Show resolved
Hide resolved
| processed.add(ent); | ||
| updatedIdealState = ent._updater.apply(idealStateCopy); | ||
| // System.out.println("After merging:" + merged); | ||
| it.remove(); |
There was a problem hiding this comment.
Is it safe to modify the queue here while we are iterating on it?
There was a problem hiding this comment.
yes, this is processed in single thread
e3e6f9a to
0bd3064
Compare
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #13976 +/- ##
============================================
- Coverage 61.75% 57.90% -3.85%
- Complexity 207 219 +12
============================================
Files 2436 2613 +177
Lines 133233 143270 +10037
Branches 20636 21995 +1359
============================================
+ Hits 82274 82963 +689
- Misses 44911 53825 +8914
- Partials 6048 6482 +434
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
5a92076 to
1522ef1
Compare
47b2e49 to
4d15842
Compare
Jackie-Jiang
left a comment
There was a problem hiding this comment.
LGTM.
Can you double check the existing logic and ensure there is nothing rely on only the current update being processed? If there are, we can consider providing 2 fashions, one allow batching and one does not
pinot-common/src/main/java/org/apache/pinot/common/utils/helix/IdealStateGroupCommit.java
Show resolved
Hide resolved
pinot-common/src/main/java/org/apache/pinot/common/utils/helix/IdealStateGroupCommit.java
Outdated
Show resolved
Hide resolved
4d15842 to
8e4edc7
Compare
I think all the logics using commit method are moved this this sync best-effort kind of commit logic. I would say this is always better than the single commit. |
8e4edc7 to
c63fe61
Compare
This reverts commit 717895b.
We observed IdealState update becomes bottleneck for large table with high throughput segments update.
This PR shamelessly borrowed the idea/logic/code from: https://github.com/apache/helix/blob/master/helix-core/src/main/java/org/apache/helix/GroupCommit.java to group commit IdealState updates.
By running the
IdealStateGroupCommitTesttest, for my local setup, 2400 updates using 400 threads end up with 8 Zookeeper Writes, this is a significant improvement to relief zookeeper pressure.