re20052 commented on code in PR #68042:
URL: https://github.com/apache/doris/pull/68042#discussion_r4025908220
##########
fe/fe-core/src/main/java/org/apache/doris/datasource/doris/RemoteDorisExternalTable.java:
##########
@@ -62,62 +62,58 @@ protected synchronized void makeSureInitialized() {
}
private RemoteOlapTable getDorisOlapTable() {
- if (!isSyncOlapTable) {
- synchronized (this) {
- if (!isSyncOlapTable) {
- try {
- isSyncOlapTable = true;
- remoteOlapTable = null;
- lastException = null; // clear previous exception
-
- List<Partition> cachedPartitions =
Lists.newArrayList(partitions);
- List<Partition> cachedTempPartitions =
Lists.newArrayList(tempPartitions);
- RemoteOlapTable olapTable =
((RemoteDorisExternalCatalog) catalog).getFeServiceClient()
- .getOlapTable(dbName, remoteName, tableId,
cachedPartitions, cachedTempPartitions);
- olapTable.setCatalog((RemoteDorisExternalCatalog)
catalog);
- olapTable.setDatabase((RemoteDorisExternalDatabase)
db);
-
- // Remove redundant nested synchronized block
- tableId = olapTable.getId();
- partitions =
Lists.newArrayList(olapTable.getPartitions());
- tempPartitions =
Lists.newArrayList(olapTable.getTempPartitions().getPartitions());
Review Comment:
Thanks for the review. I do not consider the previous locking behavior a
post-commit consistency boundary.
The old implementation explicitly intended concurrent callers to reuse an
in-flight refresh through isSyncOlapTable, remoteOlapTable, and
wait/notifyAll.
The reuse failed because makeSureInitialized() and the refresh section
competed
for the same monitor. Although makeSureInitialized() released the monitor
before
calling getDorisOlapTable(), the same running thread could immediately
reacquire
it before the other awakened threads were scheduled. It then started another
long refresh and blocked those threads again. This monitor barging caused the
repeated serial refreshes observed in production; it was not an intentional
metadata-generation mechanism.
This PR restores the original single-flight intent: the monitor only selects
a
FutureTask, while RPC and metadata reconstruction run outside it. Callers
that
overlap an unfinished refresh share its result. Completed results are not
cached, so the next non-overlapping request always starts a new refresh.
A normal sequential INSERT followed by SELECT remains correct: the metadata
refresh used during INSERT planning finishes before execution and commit, so
the
following SELECT sees a completed task and starts a new refresh. The reported
V-to-V+1 case requires an unrelated refresh and commit to overlap. Remote
Doris
Catalog does not currently guarantee linearizable metadata snapshots across
such concurrent operations or remote FE nodes. Such a guarantee would
require an
explicit metadata version or invalidation protocol, not reliance on
accidental
monitor scheduling.
Therefore, I do not plan to add the proposed generation boundary or encode
that
new consistency guarantee in this PR.
--
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]
