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 12:

(2 comments)

http://gerrit.cloudera.org:8080/#/c/21443/12/bin/start-impala-cluster.py
File bin/start-impala-cluster.py:

http://gerrit.cloudera.org:8080/#/c/21443/12/bin/start-impala-cluster.py@207
PS12, Line 207:                   "catalogd & cache it locally. The cached 
metadata gets evicted"
I don't think we generally use this short-hand in our help messages.


http://gerrit.cloudera.org:8080/#/c/21443/12/bin/start-impala-cluster.py@678
PS12, Line 678:     if options.use_local_catalog:
I think my point is not getting across completely.

Line 460 handles the catalogd args list. It looks correct: if use_local_catalog 
is true, add `-catalog_topic_mode=minimal` for catalogd.

This section in build_impalad_arg_lists handles the impalad args list. It only 
uses the `use_local_catalog` argument, so there's no need to also set 
`catalog_topic_mode` here.

Also I'd like to see these be consistent: either include an else clause in both 
places, or neither. I prefer omitting the else clauses; if we change Impala's 
default to be to use local catalog mode, then this option in 
start-impala-cluster will need to be revisited (either to update it to let 
local catalog be the default, change it to be a flag to disable local catalog, 
or remove it).



--
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: 12
Gerrit-Owner: Anshula Jain <[email protected]>
Gerrit-Reviewer: Impala Public Jenkins <[email protected]>
Gerrit-Reviewer: Michael Smith <[email protected]>
Gerrit-Comment-Date: Tue, 04 Feb 2025 21:17:04 +0000
Gerrit-HasComments: Yes

Reply via email to