Skip to content

Various cleanups#12713

Merged
dain merged 9 commits intotrinodb:masterfrom
dain:various-cleanups
Jun 10, 2022
Merged

Various cleanups#12713
dain merged 9 commits intotrinodb:masterfrom
dain:various-cleanups

Conversation

@dain
Copy link
Copy Markdown
Member

@dain dain commented Jun 7, 2022

Description

Various cleanups from recent work.

Documentation

(X) No documentation is needed.
( ) Sufficient documentation is included in this PR.
( ) Documentation PR is available with #prnumber.
( ) Documentation issue #issuenumber is filed, and can be handled later.

Release notes

(X) No release notes entries required.
( ) Release notes entries required with the following suggested text:

@dain dain requested a review from electrum June 7, 2022 02:31
@cla-bot cla-bot bot added the cla-signed label Jun 7, 2022
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.

there is nothing (obvious) which would prevent finished flag from changing to true just after checkState and before we get the metadata. Is that a problem?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

This commit is just reorganizing the existing code in the transaction manager. In this case the transaction manager is enforcing a single threaded access to this object that ensures that ensures safe access to this object. Technically, I think we could synchronize abort and commit, but I'm not 100% sure of the impact. Regardless we would do that in a separate commit

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.

getParentName?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Ya, I'm sure there is a better name, but I don't think parent is the right term. These don't really from a hierarchy. These internal, synthetic, names are really just a hack to deal with the very strange system and information schema tables.

Copy link
Copy Markdown
Member

@losipiuk losipiuk left a comment

Choose a reason for hiding this comment

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

skimmed

@dain dain force-pushed the various-cleanups branch from cc101e2 to f0ae8a8 Compare June 9, 2022 01:18
@dain dain merged commit 329786a into trinodb:master Jun 10, 2022
@dain dain deleted the various-cleanups branch June 10, 2022 03:20
@github-actions github-actions bot added this to the 386 milestone Jun 10, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Development

Successfully merging this pull request may close these issues.

3 participants