re20052 commented on code in PR #68042:
URL: https://github.com/apache/doris/pull/68042#discussion_r4025819155
##########
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());
-
- olapTable.setId(id); // change id in case of possible
conflicts
- olapTable.invalidateBackendsIfNeed();
- remoteOlapTable = olapTable;
- } catch (Exception e) {
- // Save exception for waiting threads
- lastException = e;
- LOG.warn("Failed to get remote doris olap table:
{}.{}", dbName, remoteName, e);
- throw e; // Re-throw the exception
- } finally {
- isSyncOlapTable = false;
- this.notifyAll();
- }
- return remoteOlapTable;
- }
+ FutureTask<RemoteOlapTable> refreshTask;
+ boolean shouldRun;
+ synchronized (this) {
+ if (currentRefreshTask == null || currentRefreshTask.isDone()) {
+ currentRefreshTask = new
FutureTask<>(this::loadDorisOlapTable);
+ shouldRun = true;
+ } else {
+ shouldRun = false;
}
+ refreshTask = currentRefreshTask;
}
- synchronized (this) {
- while (isSyncOlapTable) {
- try {
- this.wait();
- } catch (InterruptedException e) {
- throw new AnalysisException("interrupted while getting
doris olap table", e);
- }
- }
+ if (shouldRun) {
+ refreshTask.run();
+ }
+ return getRefreshResult(refreshTask);
+ }
- // If there is a saved exception, throw it with more details
- if (remoteOlapTable == null) {
- if (lastException != null) {
- throw new AnalysisException(
- "failed to get remote doris olap table: " +
Util.getRootCauseMessage(lastException),
- lastException);
- }
- throw new AnalysisException("failed to get remote doris olap
table");
- }
- return remoteOlapTable;
+ private RemoteOlapTable loadDorisOlapTable() {
+ try {
+ 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);
+
+ tableId = olapTable.getId();
+ partitions = Lists.newArrayList(olapTable.getPartitions());
Review Comment:
The previous implementation did not explicitly handle Error either, nor did
it provide consistent Error propagation across the refreshing and waiting
threads.
More importantly, Errors such as OutOfMemoryError or StackOverflowError
indicate a JVM-level failure rather than a recoverable metadata-refresh
failure. Once such an Error occurs, the health of the entire FE process is
already in question; preserving a specific propagation path in this method does
not provide a meaningful correctness guarantee for this API.
This PR targets lock contention during normal metadata refreshes. It does
not change metadata correctness or recoverable exception handling. Therefore,
JVM-fatal Error propagation is outside the scope of this fix and should not be
classified as a P1 issue.
--
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]
