Add debug utilities to debug builds #5649
Conversation
mkindahl
force-pushed
the
debug-waitpoint-for-debug-build
branch
2 times, most recently
from
29aacba
to
f93fa72
Compare
|
@jnidzwetzki, @pmwkaa: please review this pull request.
|
mkindahl
force-pushed
the
debug-waitpoint-for-debug-build
branch
6 times, most recently
from
1ca6468
to
94863a1
Compare
Codecov Report
@@ Coverage Diff @@
## main #5649 +/- ##
==========================================
- Coverage 87.09% 87.05% -0.05%
==========================================
Files 243 243
Lines 55967 55921 -46
Branches 12383 12364 -19
==========================================
- Hits 48746 48680 -66
- Misses 4891 4902 +11
- Partials 2330 2339 +9 see 21 files with indirect coverage changes 📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more |
| # These tests are using markers for the isolation tests (to avoid race | ||
| # conditions causing differing output), which were added after 12.7, 13.3, and | ||
| # 14.0. | ||
| if(PG_VERSION VERSION_GREATER "12.7" |
There was a problem hiding this comment.
We removed support for PG12 recenlty
There was a problem hiding this comment.
Test is harmless, but it is trivial to remove. Might be good to still keep the comment though, in case anybody want to test a build with deprecated versions.
There was a problem hiding this comment.
Leaving it for now. It is trivial to remove, but will generate weird errors if somebody tries to use it for PG12 even though it is deprecated.
| @@ -25,6 +25,7 @@ option( | |||
| ENABLE_OPTIMIZER_DEBUG | |||
| "Enable OPTIMIZER_DEBUG when building. Requires Postgres server to be built with OPTIMIZER_DEBUG." | |||
| OFF) | |||
| option(ENABLE_DEBUG_UTILS "Enable debug utilities for the extension." ON) | |||
There was a problem hiding this comment.
Do we need a separate setting? Maybe always enable them in Debug builds?
There was a problem hiding this comment.
I am also in favor of enabling that in every debug build.
There was a problem hiding this comment.
They are by default enabled, and since the functions are not defined for non-debug builds, this is how it currently behaves.
This will move the definitions of `debug_waitpoint_enable`, `debug_waitpoint_disable`, and `debug_waitpoint_id` to always be defined for debug builds and modify existing tests accordingly. This means that it is no longer necessary to generate isolation test files from templates (in most cases), and it will be straightforward to use these functions in debug builds. The debug utilities can be disabled by setting the option `ENABLE_DEBUG_UTILS` to `OFF`.
The telemetry isolation test `telemetry_iso` does not test anything and does not seem to work, so it is removed. The debug waitpoint was taken in the same session, so the waitpoint was not waited on.
mkindahl
force-pushed
the
debug-waitpoint-for-debug-build
branch
from
6af8df6
to
7a7fe48
Compare
This will move the definitions of
debug_waitpoint_enable,debug_waitpoint_disable, anddebug_waitpoint_idto always be defined for debug builds and modify existing tests accordingly.This means that it is no longer necessary to generate isolation test files from templates (in most cases), and it will be straightforward to use these functions in debug builds.
The debug utilities can be disabled by setting the option
ENABLE_DEBUG_UTILStoOFF.Disable-check: force-changelog-file, commit-count