Michael Smith has posted comments on this change. ( http://gerrit.cloudera.org:8080/24153 )
Change subject: IMPALA-14597: Initial HBO support ...................................................................... Patch Set 3: (11 comments) http://gerrit.cloudera.org:8080/#/c/24153/3//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/24153/3//COMMIT_MSG@17 PS3, Line 17: historiical runs. The key in the HBO cache is a hash of this string. In typo: historiical -> historical http://gerrit.cloudera.org:8080/#/c/24153/3//COMMIT_MSG@34 PS3, Line 34: e.g. "a IN (2, 1)" -> "a IN (1, 2)". It guarentees the expressions are typo: guarentees -> guarantees http://gerrit.cloudera.org:8080/#/c/24153/3//COMMIT_MSG@43 PS3, Line 43: partition predicates, e.g., ds > '20260331', it keeps them as-it. as-it -> as-is? http://gerrit.cloudera.org:8080/#/c/24153/3//COMMIT_MSG@46 PS3, Line 46: canonicalization stragety. In the future, we can add more strategies, stragety -> strategy http://gerrit.cloudera.org:8080/#/c/24153/3//COMMIT_MSG@105 PS3, Line 105: match, we will try the next stragety. Cardinality from the HBO stats strategy http://gerrit.cloudera.org:8080/#/c/24153/3//COMMIT_MSG@115 PS3, Line 115: that are retrived from HBO stats, e.g. retrived -> retrieved http://gerrit.cloudera.org:8080/#/c/24153/3/be/src/common/global-flags.cc File be/src/common/global-flags.cc: http://gerrit.cloudera.org:8080/#/c/24153/3/be/src/common/global-flags.cc@240 PS3, Line 240: "Default is 1GB."); This seems a little big? But it's probably fine; lite users in a small environment probably won't get anywhere near that. We could consider making it a percentage of something like mem_limit. http://gerrit.cloudera.org:8080/#/c/24153/3/be/src/common/global-flags.cc@243 PS3, Line 243: "Concurrency level for the HBO in-memory backend (InMemoryCacheBackend)."); Should note the default. http://gerrit.cloudera.org:8080/#/c/24153/3/be/src/runtime/coordinator.cc File be/src/runtime/coordinator.cc: http://gerrit.cloudera.org:8080/#/c/24153/3/be/src/runtime/coordinator.cc@a1887 PS3, Line 1887: Why remove the comment? http://gerrit.cloudera.org:8080/#/c/24153/3/be/src/service/impala-server.h File be/src/service/impala-server.h: http://gerrit.cloudera.org:8080/#/c/24153/3/be/src/service/impala-server.h@1264 PS3, Line 1264: void GetPlanNodesWithHboKeys( nit: could be a static function rather than part of the class interface. http://gerrit.cloudera.org:8080/#/c/24153/3/fe/src/main/java/org/apache/impala/service/InMemoryCacheBackend.java File fe/src/main/java/org/apache/impala/service/InMemoryCacheBackend.java: http://gerrit.cloudera.org:8080/#/c/24153/3/fe/src/main/java/org/apache/impala/service/InMemoryCacheBackend.java@97 PS3, Line 97: this(4, 1024L * 1024 * 1024); > This default seems too large to me. The danger I see is that the JVM limit Is this method used? -- To view, visit http://gerrit.cloudera.org:8080/24153 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I6ff60a8bd22c13c0ecad1198934cc96249b1015e Gerrit-Change-Number: 24153 Gerrit-PatchSet: 3 Gerrit-Owner: Quanlong Huang <[email protected]> Gerrit-Reviewer: Csaba Ringhofer <[email protected]> Gerrit-Reviewer: Impala Public Jenkins <[email protected]> Gerrit-Reviewer: Michael Smith <[email protected]> Gerrit-Reviewer: Quanlong Huang <[email protected]> Gerrit-Comment-Date: Tue, 31 Mar 2026 17:30:43 +0000 Gerrit-HasComments: Yes
