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

geo: Ability for TS to select a different txn status table based on local vs global requirements #10158

Closed
bmatican opened this issue Oct 1, 2021 · 1 comment
Assignees
Labels
area/docdb YugabyteDB core features

Comments

@bmatican
Copy link
Contributor

bmatican commented Oct 1, 2021

Context: #9980, #10157

Currently we only support one txn table. When we need to start a txn, we first need to pick a txn status tablet to execute it against. There is code on the TS side to do just that, optimizing for picking a tablet which has a leader on the respective TS.

Once we support multiple status tablets, each with their own placement rules, we will need to improve this picking logic, to select a tablet from either the global table (essentially what we do now) vs a local table. We can still keep the logic of picking a leader, of those!

Couple of other notes

  • Eventually, PG will have to flow down info in some PB / RPC, that it wants a global vs local txn tablet. Until that part comes, we can just use a simple gflag though, so we can test the behavior in advance!
  • For the global path, for simplicity, we can assume the existing system.transactions table is the global one.
  • For the local path, we need to figure out what is the actual placement of the tablet we need. We have not yet clarified what the contract should be, between PG and the TS. For simplicity, I would say we can start with the following assumption: if we need a local txn started at TS1, let's assume the placement information of TS1 is the one we match by (ie: the local TS gflags placement_cloud / placement_region / placement_zone). Then we need to match those against the placement of the various txn tablets available on this TS.
  • What is the placement of a particular tablet is known at the master, but we do not flow this to the TS yet. TBD if or how we want to do that. For simplicity though, I think we can get a prototype of this working, by just leveraging the information available in the consensus-meta (ie: check the raft config of the tablet and the placement info of all 3 peers).
  • There's also some code where we cache the txn tablets, as we currently expect them to be a fixed set. Would need to check with @mbautin or @spolitov for more details.

cc @deeps1991 also if above makes sense

@bmatican bmatican added the area/docdb YugabyteDB core features label Oct 1, 2021
@bmatican bmatican added this to Backlog in YBase features via automation Oct 1, 2021
@bmatican bmatican added this to To Do in Row Level Geo Partitioning via automation Oct 1, 2021
es1024 added a commit that referenced this issue Nov 10, 2021
…global requirements

Summary:
Changes TS logic to select transaction status tablet based on whether the
`force_global_transactions` gflag is enabled or not: if disabled, use tablets from the global
transaction table, else, use tablets from a local transaction table (error if not found
unless there are no local transaction tables at all).

Depends on D13356.

Test Plan:
Tested manually by logging status tablet ids in manual test and checking which table
they belonged to; also added test case: `ybd --cxx-test pgwrapper_geo_transactions-test`.

Reviewers: bogdan, rthallam, mbautin

Reviewed By: mbautin

Subscribers: zyu, ybase

Differential Revision: https://phabricator.dev.yugabyte.com/D13599
@es1024
Copy link
Contributor

es1024 commented Nov 10, 2021

Added with 6454f01.

@es1024 es1024 closed this as completed Nov 10, 2021
YBase features automation moved this from Backlog to Done Nov 10, 2021
Row Level Geo Partitioning automation moved this from To Do to Done Nov 10, 2021
mbautin added a commit that referenced this issue Nov 11, 2021
…ocal vs global requirements"

Summary: This reverts commit 6454f01 becase that commit breaks a lot of tests which should have been checked prior to landing it.

Test Plan: Jenkins

Reviewers: bogdan, sergei, esheng

Reviewed By: esheng

Subscribers: ybase

Differential Revision: https://phabricator.dev.yugabyte.com/D13897
es1024 added a commit that referenced this issue Nov 15, 2021
…global requirements

Summary:
Changes TS logic to select transaction status tablet based on whether the
`force_global_transactions` gflag is enabled or not: if enabled, use tablets from the global
transaction table, else, use tablets from a local transaction table (error if not found
unless there are no local transaction tables at all).

Changed from D13599 to use a shared lock during transaction status table
hash calculations and only calculate the hash as necessary (on transaction status table
creation and table alter).

Test Plan:
Tested manually by logging status tablet ids in manual test and checking which table
they belonged to; also added test case: `ybd --cxx-test pgwrapper_geo_transactions-test`.

Jenkins: urgent

Reviewers: bogdan, rthallam, mbautin

Reviewed By: mbautin

Subscribers: ybase

Differential Revision: https://phabricator.dev.yugabyte.com/D13900
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/docdb YugabyteDB core features
Development

No branches or pull requests

2 participants