KAFKA-13914: Add command line tool kafka-metadata-quorum.sh

[dengziming]

More detailed description of your change,
if necessary. The PR title and PR message become
the squashed commit message, so use a separate
comment to ping reviewers.
Add MetadataQuorumCommand to describe quorum status, I'm trying to use arg4j style command format, currently, we only support one sub-command which is "describe" and we can specify 2 arguments which are --status and --replication.

# describe quorum status
kafka-metadata-quorum.sh --bootstrap-server localhost:9092 describe --replication

ReplicaId	LogEndOffset	Lag	LastFetchTimeMs	LastCaughtUpTimeMs	Status  	
0        	10          	        0  	-1             	        -1                	                 Leader  	
1        	10          	        0  	-1             	        -1                	                 Follower	
2        	10          	        0  	-1             	        -1                	                 Follower	

kafka-metadata-quorum.sh --bootstrap-server localhost:9092 describe --status
ClusterId:                             fMCL8kv1SWm87L_Md-I2hg
LeaderId:                             3002
LeaderEpoch:                      2
HighWatermark:                  10
MaxFollowerLag:                 0
MaxFollowerLagTimeMs:   -1
CurrentVoters:                    [3000,3001,3002]
CurrentObservers:              [0,1,2]

# specify AdminClient properties
kafka-metadata-quorum.sh --bootstrap-server localhost:9092 --command-config config.properties describe

Summary of testing strategy (including rationale)
for the feature or bug fix. Unit and/or integration
tests are expected for any behaviour change and
system tests should be considered for larger changes.

MetadataQuorumCommandTest and MetadataQuorumCommandErrorTest

Committer Checklist (excluded from commit message)

• Verify design and implementation
• Verify test coverage and CI build status
• Verify documentation (including upgrade notes)

[hachikuji]

Thanks @dengziming. I am hoping to persuade @jsancio to let us get this into 3.3.

[dengziming]

Thank you for your review @hachikuji , There are some test failures: "Found 1 unexpected threads during @BeforeAll".
I will spend some time solving this failure and resolving your comments.

[hachikuji]

Looks pretty good. Left some small comments.

[hachikuji]

This raises an error if there is only one voter. I think we can just get rid of the filter? Then the max follower lag is just 0 if there is only one voter. We need another adjustment for maxFollowerLagTimeMs so that it also ends up as 0 when there is only one voter.

[dengziming]

Good catch, I also added a method testOnlyOneBrokerAndOneController for this case.

[hachikuji]

nit: it's conventional to leave off parenthesis for getters in Scala. there are a few of these in here.

[hachikuji]

Potentially we could use TransactionsCommand.prettyPrintTable to do the formatting.

[dengziming]

TransactionsCommand is in a separate module, I moved this method to Utils in the clients module and adjusted the method.

[dengziming]

Thank you for your suggestions @hachikuji , and I also updated the MetadataQuorumCommandTest to test more cases, PTAL again.

[dengziming]

Good catch, I also added a method testOnlyOneBrokerAndOneController for this case.

[dengziming]

TransactionsCommand is in a separate module, I moved this method to Utils in the clients module and adjusted the method.

[hachikuji]

Thanks, left a few more comments. Seems like there are some test failures as well.

[hachikuji]

An alternative might be to put this in server-common instead. We would have to add it as a dependency for tools, but it might be kind of nice to keep it out of clients if it's not really needed there.

[hachikuji]

Not sure if you saw my comment. I do think it is a little nicer to raise an IllegalStateException. It gives us a little protection from the future in case we break something in the parsing logic.

[hachikuji]

How about NodeId instead of ReplicaId to go along with the node.id configuration?

[hachikuji]

nit: maybe rename voter since this function is also handling observers

[hachikuji]

Perhaps it would be better not to expose this here if it is not supported by both implementations. We can still cast the ClusterInstance to RaftClusterInstance to get access to the method.

[hachikuji]

nit: is it necessary to specify controllers when the type is ZK?

[hachikuji]

Thanks, LGTM.

[hachikuji]

@dengziming It looks like we need this patch in order to stabilize the new tests: #12517. I will try to merge it today and trigger a rebuild.

[hachikuji]

@dengziming I've disabled MetadataQuorumCommandTest for now so that we can get this checked in. I see a similar issue in #12469, but I am unable to reproduce it locally. Let's follow up to try and get to the bottom of it so that we can enable the test.

[hachikuji]

@dengziming I reverted the commit to disable the test. I think we found the cause of the flakiness and fixed here: d1936ab.

[dengziming]

Thanks for your patience @hachikuji