Michael Smith has posted comments on this change. ( http://gerrit.cloudera.org:8080/21443 )
Change subject: IMPALA-12869: Shorten argument list to enable local catalog mode ...................................................................... Patch Set 4: (7 comments) http://gerrit.cloudera.org:8080/#/c/21443/4/bin/start-impala-cluster.py File bin/start-impala-cluster.py: http://gerrit.cloudera.org:8080/#/c/21443/4/bin/start-impala-cluster.py@210 PS4, Line 210: parser.add_option("--catalog_topic_mode", dest="catalog_topic_mode", This appears to be unused. It's true that most of the time we use_local_catalog=true, we want to set catalog_topic_mode=minimal. catalog-server.cc notes that 'mixed' mode exists for when some coordinators use local catalog and others don't. We would need to provide coordinator-specific config to use that, so I think we can omit it from the shortcut option. I'd remove the separate catalog_topic_mode option. http://gerrit.cloudera.org:8080/#/c/21443/4/bin/start-impala-cluster.py@266 PS4, Line 266: nit: Unnecessary whitespace. http://gerrit.cloudera.org:8080/#/c/21443/4/bin/start-impala-cluster.py@361 PS4, Line 361: nit: Unnecessary whitespace. http://gerrit.cloudera.org:8080/#/c/21443/4/bin/start-impala-cluster.py@448 PS4, Line 448: catalogd_arg_list = [set_catalog_topic_mode(options.use_local_catalog)] I think this could be simpler. Handle it the same as other options below: if options.use_local_catalog: args.extend(["-catalog_topic_mode=minimal"]) use_local_catalog is unnecessary here, it's only used by coordinators. http://gerrit.cloudera.org:8080/#/c/21443/4/bin/start-impala-cluster.py@685 PS4, Line 685: args = "{args} -use_local_catalog=true -catalog_topic_mode=minimal".format( catalog_topic_mode only needs to be set on catalogd. http://gerrit.cloudera.org:8080/#/c/21443/4/bin/start-impala-cluster.py@688 PS4, Line 688: args = "{args} -use_local_catalog=false -catalog_topic_mode=full".format( If not set, don't specify an option. Tests still set this via impalad_args. http://gerrit.cloudera.org:8080/#/c/21443/4/bin/start-impala-cluster.py@719 PS4, Line 719: def set_catalog_topic_mode(use_local_catalog): I don't understand what this is doing. -- To view, visit http://gerrit.cloudera.org:8080/21443 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Iab5f3613b39a8473ba1f6ab7cb2634d87b808142 Gerrit-Change-Number: 21443 Gerrit-PatchSet: 4 Gerrit-Owner: Anshula Jain <[email protected]> Gerrit-Reviewer: Impala Public Jenkins <[email protected]> Gerrit-Reviewer: Michael Smith <[email protected]> Gerrit-Comment-Date: Tue, 12 Nov 2024 17:59:54 +0000 Gerrit-HasComments: Yes
