Bankim Bhavsar has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/14533 )

Change subject: KUDU-2962 Fix kudu::itest::FindTabletFollowers() test utility 
function
......................................................................


Patch Set 3:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/14533/1/src/kudu/integration-tests/cluster_itest_util.cc
File src/kudu/integration-tests/cluster_itest_util.cc:

http://gerrit.cloudera.org:8080/#/c/14533/1/src/kudu/integration-tests/cluster_itest_util.cc@694
PS1, Line 694:   uuids.emplace(peer.permanent_uuid())
> In some intermittent states Raft consensus might have no leader.  I think in 
> that case this method can return Status::IllegalState().

Done without removing this DCHECK. See below.

> From the upper level, there should be a way to get eventual success from this 
> method using ASSERT_EVENTUALLY() or alike.

Done.

> There is already a check above L685 whether the tablet server returned leader 
> uuid. Is it possible that has_leader_uuid() is true but the actual the 
> leader_uuid() is empty?

Checked ConsensusMetadata::ToConsensusStatePB(), there can't be a case where 
leader is set but it's empty.

> In case of intermediate state when there is no leader for the tablet id, 
> check on L711 would return failure.

Enhanced the error checking and reporting mechanism to differentiate between 
error due to no tablet server reachable or no tablet server found hosting the 
tablet_id and no current leader or peers as per consensus.

> So looks like then I should wrap all callers of this function under 
> ASSERT_EVENTUALLY().

Done.



--
To view, visit http://gerrit.cloudera.org:8080/14533
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I94146873b09cb95c54175d16305b713365cfeaa8
Gerrit-Change-Number: 14533
Gerrit-PatchSet: 3
Gerrit-Owner: Bankim Bhavsar <[email protected]>
Gerrit-Reviewer: Alexey Serbin <[email protected]>
Gerrit-Reviewer: Bankim Bhavsar <[email protected]>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Comment-Date: Wed, 23 Oct 2019 21:38:51 +0000
Gerrit-HasComments: Yes

Reply via email to