Skip to content

Replace map entry/key type flags with an enum #362

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

Merged
merged 1 commit into from
May 28, 2025

Conversation

jeltz
Copy link
Collaborator

@jeltz jeltz commented May 23, 2025

The code was much harder to understand than necessary due to the type being encoded as a bitmask where most combinations of bits were invalid. Using an enum makes the five different states which we encoded much more obvious.

The WAL_INVALID state is a bit special since it is only ever set on the key and never on the map entry itself.

@jeltz jeltz requested review from dutow and dAdAbird as code owners May 23, 2025 19:15
@jeltz jeltz force-pushed the tde/map-entry-flags branch 3 times, most recently from ed57e0f to a6b4f5b Compare May 23, 2025 19:23
typedef enum
{
MAP_ENTRY_EMPTY = 0,
TDE_KEY_TYPE_SMGR = 1,
Copy link
Collaborator

Choose a reason for hiding this comment

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

If we are also changing the values, it is probably a good idea to also change PG_TDE_FILEMAGIC

Copy link
Collaborator

Choose a reason for hiding this comment

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

If we are also changing the values, it is probably a good idea to also change PG_TDE_FILEMAGIC

Do we need to care about existing clusters before the june release?

Copy link
Collaborator

Choose a reason for hiding this comment

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

No. I don't think that we should create an upgrade path, but it's a good practice to prevent accidents. These files might stay there even if somebody uninstalled the previous rc2 extension.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Agreed. Pushed a version bump.

The code was much harder to understand than necessary due to the type
being encoded as a bitmask where most combinations of bits were invalid.
Using an enum makes the five different states which we encoded much more
obvious.

The WAL_INVALID state is a bit special since it is only ever set on the
key and never on the map entry itself.
@jeltz jeltz force-pushed the tde/map-entry-flags branch from a6b4f5b to cebd5a3 Compare May 27, 2025 16:00
@jeltz jeltz merged commit 8f522b2 into percona:TDE_REL_17_STABLE May 28, 2025
16 checks passed
@jeltz jeltz deleted the tde/map-entry-flags branch June 4, 2025 10:22
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.

3 participants