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

Compatibility issue about multi Group TSO #6142

Closed
Tracked by #5895
nolouch opened this issue Mar 10, 2023 · 4 comments
Closed
Tracked by #5895

Compatibility issue about multi Group TSO #6142

nolouch opened this issue Mar 10, 2023 · 4 comments
Assignees
Labels
type/development The issue belongs to a development tasks

Comments

@nolouch
Copy link
Contributor

nolouch commented Mar 10, 2023

Compatibility issue about Group TSO

ref #5895

MaxTS

Problem

Async Commit use max_ts to commit. The problem is, if multi group tso is used with async commit/1pc enabled, the commit_ts calculation becomes complicated. We can't record only one max_tsany more. For example, in this case:

  • Read k1 from TSO Group1, start_ts (allocated from local allocator 1) = 100, then the max_ts is updated to 100;
  • Write k2 from TSO Group2, start_ts (allocated from local allocator 2) = 50, calculated commit_ts = 101 (max_ts + 1)
    Commit k2 from TSO Group2
  • Read k2 from TSO Group1, start_ts (allocated from local allocator 2) = 51, then the previous transaction's result is invisible.

Resolve TS

the resolve ts formula:

min_ts = pd.tso()
new_resolved_ts = min(min_ts, all(lock.start_ts))
resolved_ts = min(new_resolved_ts, resolved_ts)

Problem

If there is no locks, it maybe wrong update the resolved ts, fix with:

min_ts = min(all group.tso())
@nolouch nolouch added type/feature-request Categorizes issue or PR as related to a new feature. type/development The issue belongs to a development tasks and removed type/feature-request Categorizes issue or PR as related to a new feature. labels Mar 10, 2023
@nolouch nolouch changed the title Compatibility issue about Group TSO Compatibility issue about multi Group TSO Mar 10, 2023
@ystaticy
Copy link
Contributor

ystaticy commented Mar 10, 2023

In keyspace scenario, there is a globally unique GC Worker to calculate the globally unique GC safepoint.
The calculation of GC safepoint depends on three values:

  1. min(all keyspace service safepoint)
  2. now-10min
  3. min(all keyspace. minstartts)
    now in the second item above, it's always get from a unique timeline.
    now, err := w.getOracleTime()

But if TSO in differrent group, multiple timelines will be created.
So there is a possible options:
If TSO is in differrent group, but we still use the globally unique gc safepoint.
now should take the minimum value of all TSO groups. It need PD provide a new interface to return min(all TSO group nowTS)

Subsequent plan: the calculation of GC safepoint will be separated by keyspace,but the design discussion and code review may take more time.

@ystaticy
Copy link
Contributor

There is another problem:
In keyspace scenario, if we use the globally unique gc safepoint.
now, gc safepoint = ts1;
We need to constrain the new tso group timeline to be later than ts1.
Otherwise, the data in the new tso group will be directly gc.

@nolouch
Copy link
Contributor Author

nolouch commented May 5, 2023

cc @rleungx @binshi-bing

@rleungx
Copy link
Member

rleungx commented May 5, 2023

native_br also needs to get the global min ts.

ti-chi-bot bot pushed a commit that referenced this issue May 12, 2023
…i-timeline tso (#6421)

ref #6142

1. Import kvproto change to introduce GetMinTS rpc in the TSO service.
6. Add server side implementation for GetMinTS rpc.
7. Add client side implementation for GetMinTS rpc.
8. Add unit test

Signed-off-by: Bin Shi <binshi.bing@gmail.com>
ti-chi-bot bot added a commit that referenced this issue May 17, 2023
ref #6142

Signed-off-by: Ryan Leung <rleungx@gmail.com>

Co-authored-by: ti-chi-bot[bot] <108142056+ti-chi-bot[bot]@users.noreply.github.com>
ti-chi-bot bot pushed a commit that referenced this issue May 18, 2023
ref #6142

Signed-off-by: Ryan Leung <rleungx@gmail.com>
rleungx pushed a commit to rleungx/pd that referenced this issue Aug 2, 2023
…i-timeline tso (tikv#6421)

ref tikv#6142

1. Import kvproto change to introduce GetMinTS rpc in the TSO service.
6. Add server side implementation for GetMinTS rpc.
7. Add client side implementation for GetMinTS rpc.
8. Add unit test

Signed-off-by: Bin Shi <binshi.bing@gmail.com>
rleungx added a commit to rleungx/pd that referenced this issue Aug 2, 2023
ref tikv#6142

Signed-off-by: Ryan Leung <rleungx@gmail.com>

Co-authored-by: ti-chi-bot[bot] <108142056+ti-chi-bot[bot]@users.noreply.github.com>
rleungx added a commit to rleungx/pd that referenced this issue Aug 2, 2023
ref tikv#6142

Signed-off-by: Ryan Leung <rleungx@gmail.com>
rleungx pushed a commit to rleungx/pd that referenced this issue Aug 21, 2023
…i-timeline tso (tikv#6421)

ref tikv#6142

1. Import kvproto change to introduce GetMinTS rpc in the TSO service.
6. Add server side implementation for GetMinTS rpc.
7. Add client side implementation for GetMinTS rpc.
8. Add unit test

Signed-off-by: Bin Shi <binshi.bing@gmail.com>
@nolouch nolouch closed this as completed Oct 17, 2023
rleungx pushed a commit to rleungx/pd that referenced this issue Apr 16, 2024
…i-timeline tso (tikv#6421)

ref tikv#6142

1. Import kvproto change to introduce GetMinTS rpc in the TSO service.
6. Add server side implementation for GetMinTS rpc.
7. Add client side implementation for GetMinTS rpc.
8. Add unit test

Signed-off-by: Bin Shi <binshi.bing@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type/development The issue belongs to a development tasks
Projects
None yet
Development

No branches or pull requests

3 participants