This is an automated email from the ASF dual-hosted git repository.
yx-keith pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/doris.git
The following commit(s) were added to refs/heads/master by this push:
new 3cd43b65e5a [improvement](catalog) narrow MaterializedIndex addTablet
sync scope (#64478)
3cd43b65e5a is described below
commit 3cd43b65e5a3d201627e1cd0c79d7bfcd4c424d5
Author: yaoxiao <[email protected]>
AuthorDate: Thu Jun 25 20:15:48 2026 +0800
[improvement](catalog) narrow MaterializedIndex addTablet sync scope
(#64478)
Move the synchronized modifier from the public addTablet / appendTablets
methods onto appendTabletsInternal, the only block that actually mutates
the per-index list and idToTablets map.
The TabletInvertedIndex.addTablet(...) call in the non-restore path of
addTablet does not need to share this lock — it has its own internal
synchronization and operates on a separate object. Holding the per-index
monitor across that call only serialized unrelated writers without
guarding any shared state on this index.
Readers see the new tablets via the same volatile publish in
appendTabletsInternal as before, so the existing TOCTOU window between
the per-index snapshot and TabletInvertedIndex (handled by the
IllegalStateException catch in TabletStatMgr.updateTabletStat) is
unchanged in both width and shape.
### What problem does this PR solve?
Issue Number: close #xxx
Related PR: #63298
Problem Summary:
### Release note
None
### Check List (For Author)
- Test <!-- At least one of them must be included. -->
- [ ] Regression test
- [ ] Unit Test
- [ ] Manual test (add detailed scripts or steps below)
- [ ] No need to test or manual test. Explain why:
- [ ] This is a refactor/code format and no logic has been changed.
- [ ] Previous test can cover this change.
- [ ] No code files have been changed.
- [ ] Other reason <!-- Add your reason? -->
- Behavior changed:
- [ ] No.
- [ ] Yes. <!-- Explain the behavior change -->
- Does this need documentation?
- [ ] No.
- [ ] Yes. <!-- Add document PR link here. eg:
https://github.com/apache/doris-website/pull/1214 -->
### Check List (For Reviewer who merge this PR)
- [ ] Confirm the release note
- [ ] Confirm test cases
- [ ] Confirm document
- [ ] Add branch pick label <!-- Add branch pick label that this PR
should merge into -->
---
.../apache/doris/catalog/MaterializedIndex.java | 29 ++++++++--------------
1 file changed, 10 insertions(+), 19 deletions(-)
diff --git
a/fe/fe-core/src/main/java/org/apache/doris/catalog/MaterializedIndex.java
b/fe/fe-core/src/main/java/org/apache/doris/catalog/MaterializedIndex.java
index f84ff018ef6..74b36e31d46 100644
--- a/fe/fe-core/src/main/java/org/apache/doris/catalog/MaterializedIndex.java
+++ b/fe/fe-core/src/main/java/org/apache/doris/catalog/MaterializedIndex.java
@@ -126,37 +126,28 @@ public class MaterializedIndex extends MetaObject
implements GsonPostProcessable
idToTablets = new HashMap<>();
}
- public synchronized void addTablet(Tablet tablet, TabletMeta tabletMeta) {
+ public void addTablet(Tablet tablet, TabletMeta tabletMeta) {
addTablet(tablet, tabletMeta, false);
}
- // Writers are synchronized on this index to prevent concurrent
lost-update:
- // some callers (e.g. InternalCatalog.createTablets) do NOT hold the
OlapTable
- // write lock when adding tablets.
- // Copy-on-write keeps readers CME-safe without locking; for bulk creation
use
- // appendTablets(...) so the per-index tablets list is copied once per
batch
- // instead of once per tablet.
- public synchronized void addTablet(Tablet tablet, TabletMeta tabletMeta,
boolean isRestore) {
+ // For bulk creation, prefer appendTablets() so the per-index list is
copied
+ // once per batch instead of once per tablet.
+ public void addTablet(Tablet tablet, TabletMeta tabletMeta, boolean
isRestore) {
appendTabletsInternal(Collections.singletonList(tablet));
if (!isRestore) {
Env.getCurrentInvertedIndex().addTablet(tablet.getId(),
tabletMeta);
}
}
- // Bulk-publish: append the given tablets to this index's tablets list in a
- // single copy-on-write (O(existing + batch) instead of O(n^2) over n
- // single-tablet adds inside a synchronized block).
- //
- // Does NOT touch TabletInvertedIndex. Bulk-creation callers register
tablets
- // in TabletInvertedIndex eagerly inside their per-tablet loop because
- // Tablet.addReplica(...) (non-restore) requires the tablet to already be
- // present in the inverted index; only the per-index list copy is expensive
- // enough to be worth batching.
- public synchronized void appendTablets(Collection<Tablet> newTablets) {
+ // Bulk-publish path. Does NOT register tablets in TabletInvertedIndex —
+ // callers do that per tablet (needed before Tablet.addReplica in
non-restore paths).
+ public void appendTablets(Collection<Tablet> newTablets) {
appendTabletsInternal(newTablets);
}
- private void appendTabletsInternal(Collection<Tablet> newTablets) {
+ // Synchronized to prevent concurrent lost-update on the per-index
list/map snapshots;
+ // some callers (e.g. InternalCatalog.createTablets) do not hold the
OlapTable write lock.
+ private synchronized void appendTabletsInternal(Collection<Tablet>
newTablets) {
if (newTablets.isEmpty()) {
return;
}
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
