Riza Suminto has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/23382 )

Change subject: IMPALA-14400: Fix deadlock in 
CatalogServiceCatalog.getDbProperty()
......................................................................


Patch Set 5:

(5 comments)

http://gerrit.cloudera.org:8080/#/c/23382/4//COMMIT_MSG
Commit Message:

http://gerrit.cloudera.org:8080/#/c/23382/4//COMMIT_MSG@28
PS4, Line 28: safe for write purpose.
> nit: we might want to add it back in the future when adding more catalogTim
Ack. Reverted this.


http://gerrit.cloudera.org:8080/#/c/23382/4/fe/src/main/java/org/apache/impala/catalog/CatalogServiceCatalog.java
File fe/src/main/java/org/apache/impala/catalog/CatalogServiceCatalog.java:

http://gerrit.cloudera.org:8080/#/c/23382/4/fe/src/main/java/org/apache/impala/catalog/CatalogServiceCatalog.java@504
PS4, Line 504:       }
             :
             :       try {
> A more sensible approach might be to not require acquire write lock here to
Done


http://gerrit.cloudera.org:8080/#/c/23382/4/fe/src/main/java/org/apache/impala/catalog/CatalogServiceCatalog.java@1501
PS4, Line 1501:         // catalog state.
              :         resetManager_.waitOngoingMetadataFetch(dbName_);
              :       }
              :       return dbCache_.get
> I think we do have readers that also want to see the latest catalog state,
Patch set 5 ensure that waitInitialResetCompletion() is safe to call. 
CatalogServiceCatalog.getDb() will avoid returning null unless it is a passive 
CatalogD.


http://gerrit.cloudera.org:8080/#/c/23382/4/fe/src/main/java/org/apache/impala/catalog/CatalogServiceCatalog.java@1505
PS4, Line 1505:     }
> Previously we only do this when triggeredInitialReset_ is false. But it see
We do call waitOngoingMetadataFetch() in several places where we acquire write 
lock. This generalize it.


http://gerrit.cloudera.org:8080/#/c/23382/4/fe/src/main/java/org/apache/impala/catalog/CatalogServiceCatalog.java@2644
PS4, Line 2644:
> Can we remove this and let all waiters waiting for the read lock? Then afte
The signal is there to avoid write lock waiter to spin loop and sleep.
With change in patch set 5, we can keep this without risking deadlock.



--
To view, visit http://gerrit.cloudera.org:8080/23382
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I3162472ea9531add77886bf1d0d73460ff34d07a
Gerrit-Change-Number: 23382
Gerrit-PatchSet: 5
Gerrit-Owner: Riza Suminto <[email protected]>
Gerrit-Reviewer: Anonymous Coward <[email protected]>
Gerrit-Reviewer: Impala Public Jenkins <[email protected]>
Gerrit-Reviewer: Quanlong Huang <[email protected]>
Gerrit-Reviewer: Riza Suminto <[email protected]>
Gerrit-Comment-Date: Fri, 05 Sep 2025 15:13:12 +0000
Gerrit-HasComments: Yes

Reply via email to