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

schedule intent resolution on range with EndTransaction (in regular case) #1821

Closed
tbg opened this issue Jul 26, 2015 · 2 comments
Closed

schedule intent resolution on range with EndTransaction (in regular case) #1821

tbg opened this issue Jul 26, 2015 · 2 comments
Assignees
Labels
C-enhancement Solution expected to add code/behavior + preserve backward-compat (pg compat issues are exception)

Comments

@tbg
Copy link
Member

tbg commented Jul 26, 2015

this picks up @spencerkimball's thought of having the coordinator send along all intents with the EndTransaction call, so that the range can (in the normal case) resolve most of them instantly.
if that happens synchronously with the EndTransaction batch, the successful resolution can be reported along with the response and the coordinator can omit these intents.

this is a bit of a balancing act between introducing necessary optimizations to have intent resolution not an obvious issue in performance (see bank example) and avoiding premature introduction of complexity (intent resolution at the coordinator is still going to be relevant since transactions don't necessarily fully happen on its base range, so the txn cleanup still needs to happen in the coordinator - the coordinator also (yet) doesn't know about ranges at all).

Everything below are general musings that will find their way into follow-up issues:

this is going to be good warm-up for range-level batches: once the coordinator learns about ranges, it can chop up batches by destination range, the normal case being a Txn committing in a single round-trip without anyone ever being able to run into the intents. The resolution logic can use this so that one RPC per affected range is the maximum number needed for intent resolution. Another level of fun could use the information that the affected ranges are "usually" co-located on the same store, so the Store could actually make sure the intents are resolved (near-)synchronously. Of course that only makes sense if ranges are usually colocated on all stores that they live on (maybe once we have CopySets), certainly not in the foreseeable future.

@tbg tbg self-assigned this Jul 26, 2015
@spencerkimball
Copy link
Member

This is skipping one of the important goals of this suggested change: to clean up the transaction record.

Let's definitely resolve intents which are local to the range where the transaction is being closed as part of the same batch. But idea was to pass the remainder of non-local intents to an asynchronous processor (basically moving the cleanup code from txn_coord_sender) for cleanup and removal of the transaction record, assuming all intents are known.

@tamird tamird added the C-enhancement Solution expected to add code/behavior + preserve backward-compat (pg compat issues are exception) label Jul 27, 2015
@tbg
Copy link
Member Author

tbg commented Jul 30, 2015

@spencerkimball sorry, yes, removing the Txn record and moving the async handling down to the range is part of the suggested change. I'll move this over to RFC format and clean it up.

BTW, do we garbage-collect old txn records? Doesn't look like it so far. InternalPushTxn errors out when you try to push cases like that. It looks like you need the information about which intents may still be open along with the Txn record, so that a queue can remove those Txn records which don't have any left, re-handling the intents found on nonrecent transactions. If that sounds good we should track it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-enhancement Solution expected to add code/behavior + preserve backward-compat (pg compat issues are exception)
Projects
None yet
Development

No branches or pull requests

3 participants