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

Refactor initialization phase of transaction #790

Merged
merged 9 commits into from
Feb 14, 2023

Conversation

dranikpg
Copy link
Contributor

@dranikpg dranikpg commented Feb 13, 2023

InitByArgs is 200+ LOC and I need to extend it to support different multi-transaction modes, so I decided to refactor it a little, else it'll become an unmanageable mess

I've split it into multiple commits, each of them encompasses one or two changes

Signed-off-by: Vladislav Oleshko <vlad@dragonflydb.io>
Signed-off-by: Vladislav Oleshko <vlad@dragonflydb.io>
Signed-off-by: Vladislav Oleshko <vlad@dragonflydb.io>
Signed-off-by: Vladislav Oleshko <vlad@dragonflydb.io>
Signed-off-by: Vladislav Oleshko <vlad@dragonflydb.io>
/**
*
* There are 4 options that we consider here:
* a. T spans a single shard and its not multi.
Copy link
Collaborator

Choose a reason for hiding this comment

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

use consistent notion for transaction: for example, Tx instead of T and Trans

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I didn't touch you comments at all yet

Copy link
Collaborator

Choose a reason for hiding this comment

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

oh, I thought their are yours. in that case the comments are perfect 🙃

Signed-off-by: Vladislav Oleshko <vlad@dragonflydb.io>
Copy link
Collaborator

@romange romange left a comment

Choose a reason for hiding this comment

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

My only ask here: please try to do as little functional changes as possible for the refactoring PR,
and do all the functional changes in the subsequent PRs

@dranikpg
Copy link
Contributor Author

dranikpg commented Feb 13, 2023

There are no functional changes in this PR, except that we have a new global_ field

Signed-off-by: Vladislav Oleshko <vlad@dragonflydb.io>
Signed-off-by: Vladislav Oleshko <vlad@dragonflydb.io>
Signed-off-by: Vladislav Oleshko <vlad@dragonflydb.io>
@dranikpg dranikpg marked this pull request as ready for review February 13, 2023 19:56
@romange romange merged commit 3e46fd1 into dragonflydb:main Feb 14, 2023
@dranikpg dranikpg deleted the refactor-tx-init branch February 27, 2023 16:39
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.

2 participants