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

Fixing issues with running installcheck-world with pg_tde #31

Merged
merged 2 commits into from
Jan 22, 2025

Conversation

dutow
Copy link
Collaborator

@dutow dutow commented Jan 20, 2025

  • The make CI action now also runs the entire installcheck-world with pg_tde setup for all tests
  • The meson CI runner doesn't do this yet
  • Tools that only worked with the heap am based on an OID check now also check for the tde_heap OID
  • A few tests that do a custom server setup was disabled based on the TDE_MODE environment variable. These tests would fail because they expect that after an initdb and start, the regression suite works, but that's not the case with tde_heap. These tests can be re-enabled again after we have options to do this with initdb

@dutow dutow force-pushed the tdeworldfix branch 7 times, most recently from 3936845 to 7d6dc2b Compare January 20, 2025 12:20
* The make CI action now also runs the entire installcheck-world
  with pg_tde setup for all tests
* The meson CI runner doesn't do this yet
* Tools that only worked with the heap am based on an OID check now
  also check for the tde_heap OID
* The get_tde_table_am_oid helper function is now moved inside the core,
  as it is required by other contrib modules, which do not have access
  to the tde code otherwise.
* A few tests that do a custom server setup was disabled based on the
  TDE_MODE environment variable. These tests would fail because they
  expect that after an initdb and start, the regression suite works,
  but that's not the case with tde_heap. These tests can be re-enabled
  again after we have options to do this with initdb
Copy link
Collaborator

Choose a reason for hiding this comment

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

This will cause two server regressions to run one after other.

  • First server regression (make check-world) run will be against default heap method, and will use tmp server installation as using 'check-world'.
  • Second run will be (make installcheck-world) on last line, where we are trying to run for tde_heap.

I would suggest to move above these two jobs to separate individual jobs (adding to matrix that you are using). One should be specific to 'heap' with make installcheck-world, and second should be for 'tde_heap' with also make installcheck-world.

This way both jobs will run in parallel and will finish earlier than current implementation.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yes, but if I add another job and I don't want to build everything twice, I'll also have to split this into multiple jobs and play with artifacts and dependencies... I guess I can do that, I just wanted to keep the CI scripts simpler, as this doesn't take that long.

Copy link
Collaborator

Choose a reason for hiding this comment

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

I guess we need to use 'SELECT pg_reload_conf();' at end of file to make sure, requested changes are reflected and working before regression start.

@dutow dutow force-pushed the tdeworldfix branch 7 times, most recently from 9761091 to 466f53e Compare January 21, 2025 20:04
@dutow
Copy link
Collaborator Author

dutow commented Jan 21, 2025

@Naeem-Akhter done and done, please check

@dutow dutow requested a review from Naeem-Akhter January 21, 2025 20:49
@dutow dutow merged commit 5e3f82c into percona:TDE_REL_17_STABLE Jan 22, 2025
12 checks passed
@dutow dutow deleted the tdeworldfix branch January 22, 2025 17:32
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