Aleksandr Efimov has posted comments on this change. ( http://gerrit.cloudera.org:8080/24711 )
Change subject: IMPALA-15234: Add a way to clear the HBO cache ...................................................................... Patch Set 8: (5 comments) http://gerrit.cloudera.org:8080/#/c/24711/8/fe/src/main/java/org/apache/impala/service/HistoricalStats.java File fe/src/main/java/org/apache/impala/service/HistoricalStats.java: http://gerrit.cloudera.org:8080/#/c/24711/8/fe/src/main/java/org/apache/impala/service/HistoricalStats.java@219 PS8, Line 219: What is in the cache when this runs, and no more: > nit: It's unclear to me what this means. Could you reword this? Clarified that the clear only removes entries currently in the cache in PS9. http://gerrit.cloudera.org:8080/#/c/24711/8/fe/src/main/java/org/apache/impala/service/HistoricalStats.java@220 PS8, Line 220: without holding anything > nit: do you mean "without holding any locks"? Changed this to "without holding any locks" in PS9. http://gerrit.cloudera.org:8080/#/c/24711/8/fe/src/main/java/org/apache/impala/service/HistoricalStats.java@221 PS8, Line 221: * there says, so a query unregistering at that moment can store a run after the clear > nit: "as the TODO there says" sounds highly conversational. How about makin Reworded the concurrency note in PS9 and linked to writePlanNodeStats(). http://gerrit.cloudera.org:8080/#/c/24711/8/fe/src/test/java/org/apache/impala/analysis/AnalyzeStmtsTest.java File fe/src/test/java/org/apache/impala/analysis/AnalyzeStmtsTest.java: http://gerrit.cloudera.org:8080/#/c/24711/8/fe/src/test/java/org/apache/impala/analysis/AnalyzeStmtsTest.java@4978 PS8, Line 4978: public void TestClearHboStatsPrivileges() throws ImpalaException { > It's strange to put authorization tests here. Do we really need this test? Removed the duplicate test in PS9. AuthorizationStmtTest.testClearHboStats() covers the privilege checks. http://gerrit.cloudera.org:8080/#/c/24711/8/tests/query_test/test_hbo.py File tests/query_test/test_hbo.py: http://gerrit.cloudera.org:8080/#/c/24711/8/tests/query_test/test_hbo.py@197 PS8, Line 197: self.execute_query(": clear_hbo_stats()") > nit: can we verify the response like above? Added the response check for the second clear in PS9. -- To view, visit http://gerrit.cloudera.org:8080/24711 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ib58305e96f05ba49eb03d935765620915964308a Gerrit-Change-Number: 24711 Gerrit-PatchSet: 8 Gerrit-Owner: Aleksandr Efimov <[email protected]> Gerrit-Reviewer: Aleksandr Efimov <[email protected]> Gerrit-Reviewer: Impala Public Jenkins <[email protected]> Gerrit-Reviewer: Joe McDonnell <[email protected]> Gerrit-Reviewer: Michael Smith <[email protected]> Gerrit-Reviewer: Quanlong Huang <[email protected]> Gerrit-Comment-Date: Tue, 08 Sep 2026 13:48:02 +0000 Gerrit-HasComments: Yes
