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

Fix uninitialized variable ts_guc_bgw_log_level #6206

Merged

Conversation

fabriziomello
Copy link
Contributor

@fabriziomello fabriziomello commented Oct 17, 2023

In #6188 we introduced a new GUC timescaledb.bgw_log_level to control background workers log level, but if we missed to set the default value into the global variable leading to an assertion when checking for default values when defining it in the _guc_init process.

Fix it by properly initialize the global variable to the log_min_messages.

Disable-check: force-changelog-file

@fabriziomello fabriziomello added bug disable-auto-backport Do not automatically backport this PR or fix of this issue labels Oct 17, 2023
@fabriziomello fabriziomello self-assigned this Oct 17, 2023
@github-actions github-actions bot requested review from erimatnor and mahipv October 17, 2023 14:07
@github-actions
Copy link

@mahipv, @erimatnor: please review this pull request.

Powered by pull-review

@fabriziomello
Copy link
Contributor Author

Backtrace of the failure:

(gdb) bt
#0  __pthread_kill_implementation (no_tid=0, signo=6, threadid=139725080155968) at ./nptl/pthread_kill.c:44
#1  __pthread_kill_internal (signo=6, threadid=139725080155968) at ./nptl/pthread_kill.c:78
#2  __GI___pthread_kill (threadid=139725080155968, signo=signo@entry=6) at ./nptl/pthread_kill.c:89
#3  0x00007f1446c42476 in __GI_raise (sig=sig@entry=6) at ../sysdeps/posix/raise.c:26
#4  0x00007f1446c287f3 in __GI_abort () at ./stdlib/abort.c:79
#5  0x0000564c2ae47957 in ExceptionalCondition (conditionName=0x564c2b0853a4 "check_GUC_init(variable)", fileName=0x564c2b084107 "guc.c", lineNumber=4764) at assert.c:66
#6  0x0000564c2ae704f3 in define_custom_variable (variable=0x564c2c79b438) at guc.c:4764
#7  0x0000564c2ae70e03 in DefineCustomEnumVariable (name=0x7f1446bbe8a6 "timescaledb.bgw_log_level", short_desc=0x7f1446bbe878 "Log level for the background worker subsystem", long_desc=0x7f1446bbe800 "Log level for the scheduler and workers of the background worker subsystem. Requires configuration reload to change.", valueAddr=0x7f1446bfe89c <ts_guc_max_open_chunks_per_insert>, bootValue=19, options=0x7f1446bfa340 <loglevel_options>, context=PGC_SIGHUP, flags=0, check_hook=0x0, assign_hook=0x0, show_hook=0x0) at guc.c:5093
#8  0x00007f1446b387a9 in _guc_init () at /d/github.com/fabriziomello/timescaledb/src/guc.c:725
#9  0x00007f1446b45924 in _PG_init () at /d/github.com/fabriziomello/timescaledb/src/init.c:110
#10 0x0000564c2ae505d2 in internal_load_library (libname=0x564c2c6bf908 "/data/fabrizio/home/fabrizio/pgsql/REL_16_STABLE/lib/timescaledb-2.13.0-dev.so") at dfmgr.c:289
#11 0x0000564c2ae4ff3d in load_external_function (filename=0x7ffd3013be90 "$libdir/timescaledb-2.13.0-dev", funcname=0x7f1447f0af51 "ts_post_load_init", signalNotFound=false, filehandle=0x0) at dfmgr.c:116
#12 0x00007f1447f06ec2 in do_load (ext=0x7f1447f0f400 <extensions>) at /d/github.com/fabriziomello/timescaledb/src/loader/loader.c:835
#13 0x00007f1447f06ffc in extension_check (ext=0x7f1447f0f400 <extensions>) at /d/github.com/fabriziomello/timescaledb/src/loader/loader.c:866
#14 0x00007f1447f064c6 in post_analyze_hook (pstate=0x564c2c6bee78, query=0x564c2c6bef88, jstate=0x564c2c6bf098) at /d/github.com/fabriziomello/timescaledb/src/loader/loader.c:524
#15 0x00007f1447644bf2 in pgss_post_parse_analyze (pstate=0x564c2c6bee78, query=0x564c2c6bef88, jstate=0x564c2c6bf098) at pg_stat_statements.c:825
#16 0x0000564c2a829943 in parse_analyze_fixedparams (parseTree=0x564c2c6c18d8, sourceText=0x564c2c6cf848 "-- This file and its contents are licensed under the Apache License 2.0.\n-- Please see the included NOTICE for copyright information and\n-- LICENSE-APACHE for a copy of the license.\n\nSET LOCAL search_"..., paramTypes=0x0, numParams=0, queryEnv=0x0) at analyze.c:130
#17 0x0000564c2ac5729e in pg_analyze_and_rewrite_fixedparams (parsetree=0x564c2c6c18d8, query_string=0x564c2c6cf848 "-- This file and its contents are licensed under the Apache License 2.0.\n-- Please see the included NOTICE for copyright information and\n-- LICENSE-APACHE for a copy of the license.\n\nSET LOCAL search_"..., paramTypes=0x0, numParams=0, queryEnv=0x0) at postgres.c:688
#18 0x0000564c2a8ecc7a in execute_sql_string (sql=0x564c2c6cf848 "-- This file and its contents are licensed under the Apache License 2.0.\n-- Please see the included NOTICE for copyright information and\n-- LICENSE-APACHE for a copy of the license.\n\nSET LOCAL search_"...) at extension.c:780
#19 0x0000564c2a8edbad in execute_extension_script (extensionOid=16385, control=0x564c2c6c50b0, from_version=0x0, version=0x564c2c6c51d0 "2.13.0-dev", requiredSchemas=0x0, schemaName=0x564c2c6c5030 "public", schemaOid=2200) at extension.c:1117
#20 0x0000564c2a8eed3d in CreateExtensionInternal (extensionName=0x564c2c5e8800 "timescaledb", schemaName=0x564c2c6c5030 "public", versionName=0x564c2c6c51d0 "2.13.0-dev", cascade=false, parents=0x0, is_create=true) at extension.c:1681
#21 0x0000564c2a8ef3cf in CreateExtension (pstate=0x564c2c56a078, stmt=0x564c2c5e8820) at extension.c:1848
#22 0x0000564c2ac63497 in ProcessUtilitySlow (pstate=0x564c2c56a078, pstmt=0x564c2c5e88b0, queryString=0x564c2c5e7e38 "CREATE EXTENSION timescaledb;", context=PROCESS_UTILITY_TOPLEVEL, params=0x0, queryEnv=0x0, dest=0x564c2c5e90a0, qc=0x7ffd3013d0f0) at utility.c:1578
#23 0x0000564c2ac62290 in standard_ProcessUtility (pstmt=0x564c2c5e88b0, queryString=0x564c2c5e7e38 "CREATE EXTENSION timescaledb;", readOnlyTree=false, context=PROCESS_UTILITY_TOPLEVEL, params=0x0, queryEnv=0x0, dest=0x564c2c5e90a0, qc=0x7ffd3013d0f0) at utility.c:1078
#24 0x00007f1447f06934 in loader_process_utility_hook (pstmt=0x564c2c5e88b0, query_string=0x564c2c5e7e38 "CREATE EXTENSION timescaledb;", readonly_tree=false, context=PROCESS_UTILITY_TOPLEVEL, params=0x0, queryEnv=0x0, dest=0x564c2c5e90a0, completion_tag=0x7ffd3013d0f0) at /d/github.com/fabriziomello/timescaledb/src/loader/loader.c:639
#25 0x00007f144764596e in pgss_ProcessUtility (pstmt=0x564c2c5e88b0, queryString=0x564c2c5e7e38 "CREATE EXTENSION timescaledb;", readOnlyTree=false, context=PROCESS_UTILITY_TOPLEVEL, params=0x0, queryEnv=0x0, dest=0x564c2c5e90a0, qc=0x7ffd3013d0f0) at pg_stat_statements.c:1141
#26 0x0000564c2ac6113a in ProcessUtility (pstmt=0x564c2c5e88b0, queryString=0x564c2c5e7e38 "CREATE EXTENSION timescaledb;", readOnlyTree=false, context=PROCESS_UTILITY_TOPLEVEL, params=0x0, queryEnv=0x0, dest=0x564c2c5e90a0, qc=0x7ffd3013d0f0) at utility.c:526
#27 0x0000564c2ac5fa3d in PortalRunUtility (portal=0x564c2c6696e8, pstmt=0x564c2c5e88b0, isTopLevel=true, setHoldSnapshot=false, dest=0x564c2c5e90a0, qc=0x7ffd3013d0f0) at pquery.c:1158
#28 0x0000564c2ac5fcb2 in PortalRunMulti (portal=0x564c2c6696e8, isTopLevel=true, setHoldSnapshot=false, dest=0x564c2c5e90a0, altdest=0x564c2c5e90a0, qc=0x7ffd3013d0f0) at pquery.c:1315
#29 0x0000564c2ac5f0fd in PortalRun (portal=0x564c2c6696e8, count=9223372036854775807, isTopLevel=true, run_once=true, dest=0x564c2c5e90a0, altdest=0x564c2c5e90a0, qc=0x7ffd3013d0f0) at pquery.c:791
#30 0x0000564c2ac57cdc in exec_simple_query (query_string=0x564c2c5e7e38 "CREATE EXTENSION timescaledb;") at postgres.c:1274
#31 0x0000564c2ac5ce36 in PostgresMain (dbname=0x564c2c623708 "fabrizio", username=0x564c2c6236e8 "fabrizio") at postgres.c:4637
#32 0x0000564c2ab7c699 in BackendRun (port=0x564c2c61ba10) at postmaster.c:4464
#33 0x0000564c2ab7bf15 in BackendStartup (port=0x564c2c61ba10) at postmaster.c:4192
#34 0x0000564c2ab780f4 in ServerLoop () at postmaster.c:1782
#35 0x0000564c2ab7799e in PostmasterMain (argc=3, argv=0x564c2c54ed10) at postmaster.c:1466
#36 0x0000564c2aa2b372 in main (argc=3, argv=0x564c2c54ed10) at main.c:198

@fabriziomello fabriziomello enabled auto-merge (rebase) October 17, 2023 14:08
@mkindahl
Copy link
Contributor

Not sure if we can extend the tests to capture these kinds of issues?

In timescale#6188 we introduced a new GUC `timescaledb.bgw_log_level` to control
background workers log level, but if we missed to set the default value
into the global variable leading to an assertion when checking for
default values when defining it in the `_guc_init` process.

Fix it by properly initialize the global variable to the
`WARNING` that is the default for `log_min_messages`.
@fabriziomello fabriziomello force-pushed the fix_uninitialized_bgw_log_level branch from 24a55e6 to b05a73c Compare October 17, 2023 14:28
@fabriziomello
Copy link
Contributor Author

Not sure if we can extend the tests to capture these kinds of issues?

Hmmm... not sure because this assertion is when we start Postgres. Have a look into the backtrace #6206 (comment)

@codecov
Copy link

codecov bot commented Oct 17, 2023

Codecov Report

Merging #6206 (b05a73c) into main (f1fff68) will increase coverage by 0.11%.
The diff coverage is n/a.

@@            Coverage Diff             @@
##             main    #6206      +/-   ##
==========================================
+ Coverage   73.69%   73.81%   +0.11%     
==========================================
  Files         246      246              
  Lines       49816    49560     -256     
  Branches    12500    12420      -80     
==========================================
- Hits        36712    36582     -130     
+ Misses       7255     7153     -102     
+ Partials     5849     5825      -24     
Files Coverage Δ
src/guc.c 94.18% <ø> (ø)

... and 36 files with indirect coverage changes

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

@fabriziomello fabriziomello merged commit 4e06cd3 into timescale:main Oct 17, 2023
35 checks passed
fabriziomello added a commit to fabriziomello/timescaledb that referenced this pull request Oct 17, 2023
When defining a GUC Postgres do a cross-check between the initial value
of the C declaration associated to a GUC and its actual boot value in
assert-enabled builds.

Previous PR timescale#6206 didn't fixed it entirely and this happen just on PG16.

postgres/postgres@a73952b7
fabriziomello added a commit that referenced this pull request Oct 17, 2023
When defining a GUC Postgres do a cross-check between the initial value
of the C declaration associated to a GUC and its actual boot value in
assert-enabled builds.

Previous PR #6206 didn't fixed it entirely and this happen just on PG16.

postgres/postgres@a73952b7
jnidzwetzki pushed a commit to jnidzwetzki/timescaledb that referenced this pull request Nov 9, 2023
When defining a GUC Postgres do a cross-check between the initial value
of the C declaration associated to a GUC and its actual boot value in
assert-enabled builds.

Previous PR timescale#6206 didn't fixed it entirely and this happen just on PG16.

postgres/postgres@a73952b7
jnidzwetzki pushed a commit to jnidzwetzki/timescaledb that referenced this pull request Nov 19, 2023
When defining a GUC Postgres do a cross-check between the initial value
of the C declaration associated to a GUC and its actual boot value in
assert-enabled builds.

Previous PR timescale#6206 didn't fixed it entirely and this happen just on PG16.

postgres/postgres@a73952b7
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug disable-auto-backport Do not automatically backport this PR or fix of this issue
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants