Skip to content

Refactoring of transaction handling in tabletserver#6176

Merged
systay merged 33 commits intovitessio:masterfrom
planetscale:reserve-conn
May 25, 2020
Merged

Refactoring of transaction handling in tabletserver#6176
systay merged 33 commits intovitessio:masterfrom
planetscale:reserve-conn

Conversation

@systay
Copy link
Copy Markdown
Collaborator

@systay systay commented May 13, 2020

No description provided.

@systay systay requested a review from sougou as a code owner May 13, 2020 08:31
@harshit-gangal harshit-gangal force-pushed the reserve-conn branch 2 times, most recently from 10013fe to c7fb240 Compare May 14, 2020 10:03
Copy link
Copy Markdown
Contributor

@sougou sougou left a comment

Choose a reason for hiding this comment

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

I now understand the reasoning behind this, and I agree with the approach. Essentially, we have the following layers:
txExecutor: per-request object. Nothing more than a parameter wrapper. It could stay the way it is, if it cannot move to txEngine.
txEngine: Deals with transaction IDs.
txPool: Adds transactional semantics to statefulPool, and deals with real connections.
statefulPool: Manages transitions between the various sub-pools.

I think even the names can remain. We've used this nomenclature in all other places:
Executor: contains per-request logic and parameters.
Engine: main entry point for everything that concerns that service.
Pool: A pool of connections: and txPool is a pool that wraps statefulPool.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

rename to statefulConnectionPool, or scp.

@systay systay force-pushed the reserve-conn branch 3 times, most recently from dc831bb to 8c1c31c Compare May 22, 2020 05:15
Copy link
Copy Markdown
Contributor

@sougou sougou left a comment

Choose a reason for hiding this comment

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

Things look good at a high level. So, I'm 👍 if you've reviewed each other's work.

systay and others added 19 commits May 25, 2020 10:10
Signed-off-by: Andres Taylor <andres@planetscale.com>
Signed-off-by: Harshit Gangal <harshit@planetscale.com>
Signed-off-by: Andres Taylor <andres@planetscale.com>
Signed-off-by: Harshit Gangal <harshit@planetscale.com>
Signed-off-by: Andres Taylor <andres@planetscale.com>
…nd the tx code

Signed-off-by: Andres Taylor <andres@planetscale.com>
Signed-off-by: Andres Taylor <andres@planetscale.com>
Signed-off-by: Harshit Gangal <harshit@planetscale.com>
 * Renamed TxConnection to DedicatedConnection
 * Moved RecordQuery to TxProperties
 * Made made the release reasons an enum

Signed-off-by: Andres Taylor <andres@planetscale.com>
Signed-off-by: Andres Taylor <andres@planetscale.com>
Signed-off-by: Andres Taylor <andres@planetscale.com>
Signed-off-by: Andres Taylor <andres@planetscale.com>
Signed-off-by: Andres Taylor <andres@planetscale.com>
…nnection

Signed-off-by: Harshit Gangal <harshit@planetscale.com>
Signed-off-by: Andres Taylor <andres@planetscale.com>
Signed-off-by: Andres Taylor <andres@planetscale.com>
Signed-off-by: Andres Taylor <andres@planetscale.com>
Signed-off-by: Andres Taylor <andres@planetscale.com>
Signed-off-by: Andres Taylor <andres@planetscale.com>
systay and others added 13 commits May 25, 2020 10:10
Signed-off-by: Andres Taylor <andres@planetscale.com>
Signed-off-by: Andres Taylor <andres@planetscale.com>
Signed-off-by: Andres Taylor <andres@planetscale.com>
Signed-off-by: Andres Taylor <andres@planetscale.com>
Signed-off-by: Andres Taylor <andres@planetscale.com>
Signed-off-by: Andres Taylor <andres@planetscale.com>
Signed-off-by: Andres Taylor <andres@planetscale.com>
Signed-off-by: Andres Taylor <andres@planetscale.com>
Signed-off-by: Andres Taylor <andres@planetscale.com>
Signed-off-by: Andres Taylor <andres@planetscale.com>
Signed-off-by: Andres Taylor <andres@planetscale.com>
…_pool

Signed-off-by: Harshit Gangal <harshit@planetscale.com>
Signed-off-by: Harshit Gangal <harshit@planetscale.com>
Comment on lines 3448 to 3454
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

this file should not be changed.

Comment on lines 136 to 138
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

should not be changed.

Comment on lines 553 to 554
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

why release needs to be called after rollbackAndRelease?

Signed-off-by: Harshit Gangal <harshit@planetscale.com>
@systay systay merged commit 12e525c into vitessio:master May 25, 2020
@deepthi deepthi added this to the v7.0 milestone Jul 17, 2020
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.

4 participants