Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ReadIndex service may got duplicate `ctx` #4764

Closed
nolouch opened this issue May 24, 2019 · 9 comments
Assignees
Labels

Comments

@nolouch
Copy link
Contributor

@nolouch nolouch commented May 24, 2019

Bug Report

What version of TiKV are you using?
release3.0 rc2

Problems

  1. The ctx of pending reading is independently produced by tikv itself, it may same in leader and follower at the same time, the ReadIndex request will conflict and may lose one.
  2. If we use follwer_id in ctx, it will solve problem 1, but if follower restart will also produce the same ctx.
  3. advance the pending reads may need to iterate all pending reads, which is slowly. likely get the order of ctx in follower from ready is 1 2 4 5 6 7 8 9 3.
@nolouch nolouch self-assigned this May 24, 2019
@overvenus

This comment has been minimized.

Copy link
Contributor

@overvenus overvenus commented Aug 5, 2019

How about combining peer id and current unix timestamp? It does the best effort and should cover for most cases.

@nolouch nolouch assigned 5kbpers and unassigned nolouch Aug 6, 2019
@nolouch

This comment has been minimized.

Copy link
Contributor Author

@nolouch nolouch commented Aug 6, 2019

@5kbpers

This comment has been minimized.

Copy link
Contributor

@5kbpers 5kbpers commented Aug 6, 2019

How about combining peer id and current unix timestamp? It does the best effort and should cover for most cases.

Great idea. I think we could get a monotonic clock time, combine with peer_id as the first id, then increase it to allocate the following ids.

@BusyJay

This comment has been minimized.

Copy link
Contributor

@BusyJay BusyJay commented Aug 6, 2019

Monotonic clock time is not reliable. It's not guaranteed to be increased monotonically between booting.

@5kbpers

This comment has been minimized.

Copy link
Contributor

@5kbpers 5kbpers commented Aug 6, 2019

Monotonic clock time is not reliable. It's not guaranteed to be increased monotonically between booting.

Yes, we can use SystemTime too, but I'm worried about it would be not reliable when the process restart.

@BusyJay

This comment has been minimized.

Copy link
Contributor

@BusyJay BusyJay commented Aug 6, 2019

Why not use UUID?

@5kbpers

This comment has been minimized.

Copy link
Contributor

@5kbpers 5kbpers commented Aug 6, 2019

UUID can not be sorted :(

@5kbpers

This comment has been minimized.

Copy link
Contributor

@5kbpers 5kbpers commented Aug 6, 2019

Seems it is not required to be sorted (get it after reading the code again), UUID is a good idea :)

@Hoverbear

This comment has been minimized.

Copy link
Member

@Hoverbear Hoverbear commented Sep 11, 2019

Hi folks seems this is fixed by #5213. I'm closing this but feel free to reopen.

@Hoverbear Hoverbear closed this Sep 11, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
5 participants
You can’t perform that action at this time.