Jason Fehr has posted comments on this change. ( http://gerrit.cloudera.org:8080/23194 )
Change subject: IMPALA-14220 (part 2): Delay AcceptRequest until catalog is stable ...................................................................... Patch Set 8: Code-Review+1 (8 comments) http://gerrit.cloudera.org:8080/#/c/23194/6//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/23194/6//COMMIT_MSG@26 PS6, Line 26: (Invalidate Metadata) initiated by catalog-server.cc. > Replaced this hack with tracking the number of resets that have started. Done http://gerrit.cloudera.org:8080/#/c/23194/6/be/src/catalog/catalog-server.h File be/src/catalog/catalog-server.h: http://gerrit.cloudera.org:8080/#/c/23194/6/be/src/catalog/catalog-server.h@144 PS6, Line 144: bool WaitCatalogReadinessForWorkloadManagement(); > Removed this since we changed the implementation to track num of started ca Done http://gerrit.cloudera.org:8080/#/c/23194/6/be/src/catalog/catalog-server.cc File be/src/catalog/catalog-server.cc: http://gerrit.cloudera.org:8080/#/c/23194/6/be/src/catalog/catalog-server.cc@382 PS6, Line 382: class CatalogServiceThriftIf : public CatalogServiceIf { > It's indeed a hack to help requests checking if catalog reset in JVM has ac Done http://gerrit.cloudera.org:8080/#/c/23194/6/be/src/catalog/catalog-server.cc@803 PS6, Line 803: has_waited = true; > Done. But do we have any guidance for blank lines in our code style doc? Noting overly specific -- https://google.github.io/styleguide/cppguide.html#Vertical_Whitespace I thought there was a style guideline to insert a blank line after variable declarations that come first in a function. Apparently it's more of a convention that has developed over time. http://gerrit.cloudera.org:8080/#/c/23194/6/be/src/catalog/catalog-server.cc@816 PS6, Line 816: "catalogd $0 is in standby mode", > Done Done http://gerrit.cloudera.org:8080/#/c/23194/6/be/src/catalog/catalog-server.cc@819 PS6, Line 819: > This is changed in the first patch: https://gerrit.cloudera.org/c/23168/ Done http://gerrit.cloudera.org:8080/#/c/23194/6/be/src/catalog/catalog-server.cc@827 PS6, Line 827: if (current_num_reset_starts >= min_catalog_resets_to_serve) break; > It's a little bit tricky here. We are kind of spliting HA related responsib Thanks for the explanation. http://gerrit.cloudera.org:8080/#/c/23194/6/be/src/catalog/workload-management-init.cc File be/src/catalog/workload-management-init.cc: http://gerrit.cloudera.org:8080/#/c/23194/6/be/src/catalog/workload-management-init.cc@516 PS6, Line 516: return is_active_.Load(); > This has been changed back to AtomicBool and I think it's ok to read this A Done -- To view, visit http://gerrit.cloudera.org:8080/23194 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I370d21319335318e441ec3c3455bac4227803900 Gerrit-Change-Number: 23194 Gerrit-PatchSet: 8 Gerrit-Owner: Riza Suminto <[email protected]> Gerrit-Reviewer: Csaba Ringhofer <[email protected]> Gerrit-Reviewer: Impala Public Jenkins <[email protected]> Gerrit-Reviewer: Jason Fehr <[email protected]> Gerrit-Reviewer: Quanlong Huang <[email protected]> Gerrit-Reviewer: Riza Suminto <[email protected]> Gerrit-Reviewer: Wenzhe Zhou <[email protected]> Gerrit-Comment-Date: Mon, 28 Jul 2025 18:55:20 +0000 Gerrit-HasComments: Yes
