zclllyybb commented on code in PR #65476:
URL: https://github.com/apache/doris/pull/65476#discussion_r3632155142
##########
regression-test/suites/dictionary_p0/test_create_drop_sync.groovy:
##########
@@ -105,4 +132,40 @@ suite('test_create_drop_sync') {
LAYOUT(HASH_MAP)
properties('data_lifetime'='600');
"""
-}
\ No newline at end of file
+
+ waitAllDictionariesReady()
+ long originalVersion = (sql "SHOW DICTIONARIES")[0][3].toLong()
+ def commitFailureFuture
+ try {
+
GetDebugPoint().enableDebugPointForAllFEs('DictionaryManager.dataLoad.blockBeforeCommit')
Review Comment:
Fixed. Both dictionary debug points are now disabled in the same outer
finally that covers setup, waiting, DROP, and future completion, so an early
assertion or timeout cannot leak forceFailure.
##########
fe/fe-core/src/main/java/org/apache/doris/dictionary/DictionaryManager.java:
##########
@@ -282,6 +285,19 @@ private boolean hasDictionaryWithoutLock(String dbName,
String dictName) {
return dbDictIds != null && dbDictIds.containsKey(dictName);
}
+ public boolean isCurrentDictionary(Dictionary dictionary) {
+ lockRead();
Review Comment:
Fixed. The unnecessary dictionary-manager read lock was removed;
isCurrentDictionary now relies on the ConcurrentMap lookup and object identity
only.
##########
fe/fe-core/src/main/java/org/apache/doris/dictionary/DictionaryManager.java:
##########
@@ -556,45 +643,109 @@ private boolean commitNowVersion(ConnectContext ctx,
Dictionary dictionary) {
if (status.getErrorCode() != TStatusCode.OK) {
LOG.warn("Failed to commit dictionary " +
dictionary.getId() + " on be " + be.getAddress()
+ " because " + status.getErrorMsg());
- allSucceed = false;
+ allSucceeded = false;
+ } else {
+ anySucceeded = true;
}
} else {
LOG.warn("Failed to commit dictionary " +
dictionary.getId() + " on be " + be.getAddress());
- allSucceed = false;
+ allSucceeded = false;
+ outcomeUnknown = true;
}
+ } catch (Exception e) {
+ dictionary.setLastUpdateResult("commit failed: " +
e.getMessage());
+ LOG.warn("Failed to commit dictionary {} on BE {}",
dictionary.getId(), be.getId(), e);
+ allSucceeded = false;
+ outcomeUnknown = true;
}
- } catch (Exception e) {
- dictionary.setLastUpdateResult("commit failed: " + e.getMessage());
- LOG.warn("Failed to commit dictionary " + dictionary.getId(), e);
- allSucceed = false;
}
- return allSucceed;
+ return new CommitResult(allSucceeded, anySucceeded, outcomeUnknown);
}
- // abort could to all BE. swallow any failures.
- private void abortSpecificVersion(ConnectContext ctx, Dictionary
dictionary, long versionId) {
+ private void abortFailedLoad(ConnectContext ctx, Dictionary dictionary,
DictionaryStatus oldStatus) {
+ if (ctx.getStatementContext() == null) {
+ dictionary.trySetStatus(oldStatus);
+ return;
+ }
+ boolean partialLoad =
ctx.getStatementContext().isPartialLoadDictionary();
+ long stagedVersion = partialLoad
+ ? dictionary.getVersion() : dictionary.getVersion() + 1;
+ boolean versionFenced = false;
+ if (!partialLoad &&
ctx.getStatementContext().getUsedBackendsDistributing() != null
+ &&
!ctx.getStatementContext().getUsedBackendsDistributing().isEmpty()) {
+ // Persist the fence before abort so failover cannot reuse a
version targeted by a delayed RPC.
+ lockRead();
+ try {
+ if (isCurrentDictionary(dictionary)) {
+ dictionary.increaseVersion();
+
Env.getCurrentEnv().getEditLog().logDictionaryIncVersion(dictionary);
+ versionFenced = true;
+ }
+ } finally {
+ unlockRead();
+ }
+ }
+
+ AbortResult abortResult = abortSpecificVersion(ctx, dictionary,
stagedVersion);
+ lockRead();
+ try {
+ if (isCurrentDictionary(dictionary)) {
+ if (versionFenced && !abortResult.outcomeUnknown) {
+ dictionary.decreaseVersion();
Review Comment:
Fixed. Cancel and abort acknowledgements are no longer treated as proof that
late EOS is fenced. A used full-load version remains monotonically consumed, so
a retry uses V+2 and cannot be overwritten by delayed V+1 sink work.
##########
fe/fe-core/src/main/java/org/apache/doris/nereids/trees/plans/commands/insert/InsertIntoTableCommand.java:
##########
@@ -354,6 +411,37 @@ public void beforeComplete(AbstractInsertExecutor
insertExecutor, StmtExecutor e
throw new AnalysisException("Insert plan failed. Could not get target
table lock.");
}
+ private void lockCurrentTarget(ConnectContext ctx, DatabaseIf<?> database,
TableIf table) {
+ if (!(database instanceof Database) || table instanceof Dictionary) {
+ table.readLock();
Review Comment:
This is a valid existing limitation, but it cannot be made correct by the
local wrapper lock or ID check: external wrappers use deterministic
name-derived IDs, and RemoteOlapTable does not carry a remote object-generation
token. This PR therefore keeps existing external-table semantics and scopes the
strong object-identity guarantee to InternalCatalog. A complete fix needs
connector-level generation metadata propagated into the planned sink. I am
leaving this thread unresolved rather than claiming the local check solves it.
##########
fe/fe-core/src/main/java/org/apache/doris/planner/GroupCommitPlanner.java:
##########
@@ -193,26 +194,34 @@ public static void
executeGroupCommitInsert(ConnectContext ctx, PreparedStatemen
StatementContext statementContext) throws Exception {
PrepareCommand prepareCommand = preparedStmtCtx.command;
InsertIntoTableCommand command = (InsertIntoTableCommand)
(prepareCommand.getLogicalPlan());
- OlapTable table = (OlapTable) command.getTable(ctx);
for (int retry = 0; retry < MAX_RETRY; retry++) {
- if
(Env.getCurrentEnv().getGroupCommitManager().isBlock(table.getId())) {
- String msg = "insert table " + table.getId() + SCHEMA_CHANGE;
- LOG.info(msg);
- throw new DdlException(msg);
- }
boolean reuse = false;
GroupCommitPlanner groupCommitPlanner;
- if (preparedStmtCtx.groupCommitPlanner.isPresent()
- && table.getId() ==
preparedStmtCtx.groupCommitPlanner.get().table.getId()
- && table.getBaseSchemaVersion() ==
preparedStmtCtx.groupCommitPlanner.get().baseSchemaVersion) {
- groupCommitPlanner = preparedStmtCtx.groupCommitPlanner.get();
- reuse = true;
+ OlapTable currentTable = (OlapTable) command.getTable(ctx);
+ checkGroupCommitBlocked(currentTable);
Review Comment:
Fixed. Prepared group commit now acquires ref-counted admission atomically
with exact target validation and holds it through planner reuse/build and the
BE RPC. DDL blocks new admissions, waits for active requests, then drains WAL
before switching metadata. Follower BE selection is also rejected by the master
while the table is blocked.
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
