Csaba Ringhofer has posted comments on this change. ( http://gerrit.cloudera.org:8080/21072 )
Change subject: IMPALA-12831: Fix HdfsTable.toMinimalTCatalogObject() failed by concurrent modification ...................................................................... Patch Set 2: (2 comments) http://gerrit.cloudera.org:8080/#/c/21072/1/fe/src/main/java/org/apache/impala/catalog/HdfsTable.java File fe/src/main/java/org/apache/impala/catalog/HdfsTable.java: http://gerrit.cloudera.org:8080/#/c/21072/1/fe/src/main/java/org/apache/impala/catalog/HdfsTable.java@2133 PS1, Line 2133: /** > We recently fixed a performance regression on INVALIDATE that it triggers t As far as I understand even with ConcurrentHashMap we could have issues when iterating over the map while there are inserts/deletes on another thread. >adding synchronization here means INVALIDATE could again be blocked by >concurrent DDLs. The idea is to add a lock that is taken for a much shorter time, so there shouldn't be any RPCs while it is locked. The goal is solely to allow iterating over the partition map when the table level lock is not taken. So it should block invalidate only for a minimal amount of time. My problem with the current solution is not the memory usage, but the added complexity of another workaround in the catalog. Adding a new lock would also add complexity, but it wouldn't leak outside the HdfsTable class. I didn't think through the effect of partition ID changes though (or other members needed in toMinimalTHdfsPartition). http://gerrit.cloudera.org:8080/#/c/21072/2/tests/custom_cluster/test_concurrent_ddls.py File tests/custom_cluster/test_concurrent_ddls.py: http://gerrit.cloudera.org:8080/#/c/21072/2/tests/custom_cluster/test_concurrent_ddls.py@219 PS2, Line 219: h > flake8: F821 undefined name 'handle' this still uses the old version -- To view, visit http://gerrit.cloudera.org:8080/21072 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ie9f8e65c0bd24000241eedf8ca765c1e4e14fdb3 Gerrit-Change-Number: 21072 Gerrit-PatchSet: 2 Gerrit-Owner: Quanlong Huang <[email protected]> Gerrit-Reviewer: Anonymous Coward <[email protected]> Gerrit-Reviewer: Csaba Ringhofer <[email protected]> Gerrit-Reviewer: Impala Public Jenkins <[email protected]> Gerrit-Reviewer: Quanlong Huang <[email protected]> Gerrit-Reviewer: Sai Hemanth Gantasala <[email protected]> Gerrit-Comment-Date: Tue, 27 Feb 2024 10:49:49 +0000 Gerrit-HasComments: Yes
