Skip to content

log: group replica selector logging and split not leader errors#929

Merged
cfzjywxk merged 2 commits intotikv:masterfrom
cfzjywxk:error-log-split-notleader
Aug 24, 2023
Merged

log: group replica selector logging and split not leader errors#929
cfzjywxk merged 2 commits intotikv:masterfrom
cfzjywxk:error-log-split-notleader

Conversation

@cfzjywxk
Copy link
Contributor

@cfzjywxk cfzjywxk commented Aug 7, 2023

  1. Refactor and group the logging-related logic into RegionRequestSender and replicaSelector.
  2. Split the totalErros into details for NotLeader error responses,
    from
[total-region-errors="not_leader-nil:3"]
[Prefix: target peer id]-not_leader-[Suffix: Leader information in the returned error and nil if no leader]
[total-region-errors="6-not_leader-nil:1, 4-not_leader-nil:1, 5-not_leader-nil:1"]

@cfzjywxk
Copy link
Contributor Author

cfzjywxk commented Aug 7, 2023

/cc @crazycs520 PTAL

replica.store.storeID,
replica.isEpochStale(),
replica.attempts,
replica.epoch,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we need to use atomic load on this variable?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the reminder. I understand the replica should not be used concurrently as its lifetime is the same as the replicaSelector, but I forget why atomic operations are used for the epoch updating.

@cfzjywxk cfzjywxk force-pushed the error-log-split-notleader branch from 557c9e6 to 826a612 Compare August 11, 2023 09:00
Copy link
Contributor

@you06 you06 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

rest LGTM

}
}

func (s *testRegionRequestToThreeStoresSuite) TestLogging() {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Seems a manual test?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes.

Signed-off-by: cfzjywxk <lsswxrxr@163.com>
Signed-off-by: cfzjywxk <lsswxrxr@163.com>
@cfzjywxk cfzjywxk force-pushed the error-log-split-notleader branch from 826a612 to 9f88a2c Compare August 24, 2023 07:46
@cfzjywxk cfzjywxk merged commit fc88757 into tikv:master Aug 24, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants