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

Add debug utilities to debug builds #5649

Merged
merged 2 commits into from
Aug 14, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
1 change: 1 addition & 0 deletions CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Copy link
Member

Choose a reason for hiding this comment

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

Do we need a separate setting? Maybe always enable them in Debug builds?

Copy link
Contributor

Choose a reason for hiding this comment

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

I am also in favor of enabling that in every debug build.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

They are by default enabled, and since the functions are not defined for non-debug builds, this is how it currently behaves.


# Option to enable assertions. Note that if we include headers from a PostgreSQL
# build that has assertions enabled, we might inherit that setting without
Expand Down
4 changes: 4 additions & 0 deletions cmake/ScriptFiles.cmake
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,10 @@ set(SOURCE_FILES
cagg_migrate.sql
job_error_log_retention.sql)

if(ENABLE_DEBUG_UTILS AND CMAKE_BUILD_TYPE MATCHES Debug)
list(APPEND SOURCE_FILES debug_utils.sql)
endif()

if(USE_TELEMETRY)
list(APPEND SOURCE_FILES with_telemetry.sql)
else()
Expand Down
2 changes: 1 addition & 1 deletion scripts/docker-build.sh
Original file line number Diff line number Diff line change
Expand Up @@ -79,7 +79,7 @@ build_timescaledb()
if ! docker exec -u root ${BUILD_CONTAINER_NAME} /bin/bash -c " \
cd /build/debug \
&& git config --global --add safe.directory /src \
&& cmake -DGENERATE_DOWNGRADE_SCRIPT=${GENERATE_DOWNGRADE_SCRIPT} -DUSE_OPENSSL=${USE_OPENSSL} -DCMAKE_BUILD_TYPE=${BUILD_TYPE} /src \
&& cmake -DGENERATE_DOWNGRADE_SCRIPT=${GENERATE_DOWNGRADE_SCRIPT} -DENABLE_DEBUG_UTILS=off -DUSE_OPENSSL=${USE_OPENSSL} -DCMAKE_BUILD_TYPE=${BUILD_TYPE} /src \
&& make && make install \
&& echo \"shared_preload_libraries = 'timescaledb'\" >> /usr/local/share/postgresql/postgresql.conf.sample \
&& echo \"timescaledb.telemetry_level=off\" >> /usr/local/share/postgresql/postgresql.conf.sample \
Expand Down
15 changes: 15 additions & 0 deletions sql/debug_utils.sql
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
-- This file and its contents are licensed under the Apache License 2.0.
-- Please see the included NOTICE for copyright information and
-- LICENSE-APACHE for a copy of the license.

-- This file contains utility functions that are only used in debug
-- builds for debugging and testing.

CREATE OR REPLACE FUNCTION debug_waitpoint_enable(TEXT) RETURNS VOID LANGUAGE C VOLATILE STRICT
AS '@MODULE_PATHNAME@', 'ts_debug_point_enable';

CREATE OR REPLACE FUNCTION debug_waitpoint_release(TEXT) RETURNS VOID LANGUAGE C VOLATILE STRICT
AS '@MODULE_PATHNAME@', 'ts_debug_point_release';

CREATE OR REPLACE FUNCTION debug_waitpoint_id(TEXT) RETURNS BIGINT LANGUAGE C VOLATILE STRICT
AS '@MODULE_PATHNAME@', 'ts_debug_point_id';
36 changes: 14 additions & 22 deletions test/expected/test_utils.out
Original file line number Diff line number Diff line change
Expand Up @@ -27,34 +27,26 @@ ERROR: TestFailure | (big_double == small_double) [923423478.324200 == 324.3000
--
\set ECHO all
\c :TEST_DBNAME :ROLE_SUPERUSER
CREATE OR REPLACE FUNCTION debug_point_enable(TEXT)
RETURNS VOID
AS :MODULE_PATHNAME, 'ts_debug_point_enable'
LANGUAGE C VOLATILE STRICT;
CREATE OR REPLACE FUNCTION debug_point_release(TEXT)
RETURNS VOID
AS :MODULE_PATHNAME, 'ts_debug_point_release'
LANGUAGE C VOLATILE STRICT;
-- debug point already enabled
SELECT debug_point_enable('test_debug_point');
debug_point_enable
--------------------
SELECT debug_waitpoint_enable('test_debug_point');
debug_waitpoint_enable
------------------------

(1 row)

\set ON_ERROR_STOP 0
SELECT debug_point_enable('test_debug_point');
SELECT debug_waitpoint_enable('test_debug_point');
ERROR: debug point "test_debug_point" already enabled
\set ON_ERROR_STOP 1
SELECT debug_point_release('test_debug_point');
debug_point_release
---------------------
SELECT debug_waitpoint_release('test_debug_point');
debug_waitpoint_release
-------------------------

(1 row)

-- debug point not enabled
\set ON_ERROR_STOP 0
SELECT debug_point_release('test_debug_point');
SELECT debug_waitpoint_release('test_debug_point');
WARNING: you don't own a lock of type ExclusiveLock
ERROR: cannot release debug point "test_debug_point"
\set ON_ERROR_STOP 1
Expand All @@ -71,19 +63,19 @@ SELECT test_error_injection('test_error');

(1 row)

SELECT debug_point_enable('test_error');
debug_point_enable
--------------------
SELECT debug_waitpoint_enable('test_error');
debug_waitpoint_enable
------------------------

(1 row)

\set ON_ERROR_STOP 0
SELECT test_error_injection('test_error');
ERROR: error injected at debug point 'test_error'
\set ON_ERROR_STOP 1
SELECT debug_point_release('test_error');
debug_point_release
---------------------
SELECT debug_waitpoint_release('test_error');
debug_waitpoint_release
-------------------------

(1 row)

Expand Down
15 changes: 3 additions & 12 deletions test/isolation/specs/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -12,21 +12,12 @@ file(REMOVE ${ISOLATION_TEST_SCHEDULE})

set(TEST_TEMPLATES)

set(TEST_TEMPLATES_DEBUG
dropchunks_race.spec.in multi_transaction_indexing.spec.in
concurrent_query_and_drop_chunks.spec.in concurrent_add_dimension.spec.in)

if(CMAKE_BUILD_TYPE MATCHES Debug)
list(APPEND TEST_TEMPLATES ${TEST_TEMPLATES_DEBUG})
list(APPEND TEST_FILES concurrent_add_dimension.spec
concurrent_query_and_drop_chunks.spec dropchunks_race.spec
multi_transaction_indexing.spec)
endif(CMAKE_BUILD_TYPE MATCHES Debug)

foreach(TEMPLATE_FILE ${TEST_TEMPLATES})
get_filename_component(TEMPLATE ${TEMPLATE_FILE} NAME_WE)
set(TEST_FILE ${TEMPLATE}.spec)
configure_file(${TEMPLATE_FILE} ${TEST_FILE})
list(APPEND TEST_FILES "${TEST_FILE}")
endforeach(TEMPLATE_FILE)

foreach(TEST_FILE ${TEST_FILES})
string(REGEX REPLACE "(.+)\.spec" "\\1" TESTS_TO_RUN ${TEST_FILE})
file(APPEND ${ISOLATION_TEST_SCHEDULE} "test: ${TESTS_TO_RUN}\n")
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,12 +3,6 @@
# LICENSE-APACHE for a copy of the license.

setup {
CREATE OR REPLACE FUNCTION debug_waitpoint_enable(TEXT) RETURNS VOID LANGUAGE C VOLATILE STRICT
AS '@TS_MODULE_PATHNAME@', 'ts_debug_point_enable';

CREATE OR REPLACE FUNCTION debug_waitpoint_release(TEXT) RETURNS VOID LANGUAGE C VOLATILE STRICT
AS '@TS_MODULE_PATHNAME@', 'ts_debug_point_release';

DROP TABLE IF EXISTS dim_test;
CREATE TABLE dim_test(time TIMESTAMPTZ, device int, device2 int);
SELECT table_name FROM create_hypertable('dim_test', 'time', chunk_time_interval => INTERVAL '1 day');
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,12 +7,6 @@ setup {
CREATE TABLE measurements (time timestamptz, device int, temp float);
SELECT create_hypertable('measurements', 'time', 'device', 2);
INSERT INTO measurements VALUES ('2020-01-03 10:30', 1, 1.0), ('2021-01-03 10:30', 2, 2.0);

CREATE OR REPLACE FUNCTION debug_waitpoint_enable(TEXT) RETURNS VOID LANGUAGE C VOLATILE STRICT
AS '@TS_MODULE_PATHNAME@', 'ts_debug_point_enable';

CREATE OR REPLACE FUNCTION debug_waitpoint_release(TEXT) RETURNS VOID LANGUAGE C VOLATILE STRICT
AS '@TS_MODULE_PATHNAME@', 'ts_debug_point_release';
}

teardown {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,12 +7,6 @@ setup {
CREATE TABLE dropchunks_race_t1 (time timestamptz, device int, temp float);
SELECT create_hypertable('dropchunks_race_t1', 'time', 'device', 2);
INSERT INTO dropchunks_race_t1 VALUES ('2020-01-03 10:30', 1, 32.2);

CREATE OR REPLACE FUNCTION debug_waitpoint_enable(TEXT) RETURNS VOID LANGUAGE C VOLATILE STRICT
AS '@TS_MODULE_PATHNAME@', 'ts_debug_point_enable';

CREATE OR REPLACE FUNCTION debug_waitpoint_release(TEXT) RETURNS VOID LANGUAGE C VOLATILE STRICT
AS '@TS_MODULE_PATHNAME@', 'ts_debug_point_release';
}

teardown {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,12 +10,6 @@ setup {
(21, 19.5, 3);

CREATE TABLE barrier(i INTEGER);

CREATE OR REPLACE FUNCTION debug_waitpoint_enable(TEXT) RETURNS VOID LANGUAGE C VOLATILE STRICT
AS '@TS_MODULE_PATHNAME@', 'ts_debug_point_enable';

CREATE OR REPLACE FUNCTION debug_waitpoint_release(TEXT) RETURNS VOID LANGUAGE C VOLATILE STRICT
AS '@TS_MODULE_PATHNAME@', 'ts_debug_point_release';
}

teardown {
Expand Down
22 changes: 6 additions & 16 deletions test/sql/test_utils.sql
Original file line number Diff line number Diff line change
Expand Up @@ -27,26 +27,16 @@ SELECT test.double_eq();
\set ECHO all

\c :TEST_DBNAME :ROLE_SUPERUSER
CREATE OR REPLACE FUNCTION debug_point_enable(TEXT)
RETURNS VOID
AS :MODULE_PATHNAME, 'ts_debug_point_enable'
LANGUAGE C VOLATILE STRICT;

CREATE OR REPLACE FUNCTION debug_point_release(TEXT)
RETURNS VOID
AS :MODULE_PATHNAME, 'ts_debug_point_release'
LANGUAGE C VOLATILE STRICT;

-- debug point already enabled
SELECT debug_point_enable('test_debug_point');
SELECT debug_waitpoint_enable('test_debug_point');
\set ON_ERROR_STOP 0
SELECT debug_point_enable('test_debug_point');
SELECT debug_waitpoint_enable('test_debug_point');
\set ON_ERROR_STOP 1
SELECT debug_point_release('test_debug_point');
SELECT debug_waitpoint_release('test_debug_point');

-- debug point not enabled
\set ON_ERROR_STOP 0
SELECT debug_point_release('test_debug_point');
SELECT debug_waitpoint_release('test_debug_point');
\set ON_ERROR_STOP 1

-- error injections
Expand All @@ -59,12 +49,12 @@ SET ROLE :ROLE_DEFAULT_PERM_USER;

SELECT test_error_injection('test_error');

SELECT debug_point_enable('test_error');
SELECT debug_waitpoint_enable('test_error');
\set ON_ERROR_STOP 0
SELECT test_error_injection('test_error');
\set ON_ERROR_STOP 1

SELECT debug_point_release('test_error');
SELECT debug_waitpoint_release('test_error');
SELECT test_error_injection('test_error');

-- Test Scanner
Expand Down
8 changes: 0 additions & 8 deletions tsl/test/expected/remote_txn.out
Original file line number Diff line number Diff line change
Expand Up @@ -40,14 +40,6 @@ LANGUAGE C;
-- Don't forget to release lock via debug_waitpoint_release on the access node
-- since it's a session level advisory lock
--
CREATE OR REPLACE FUNCTION debug_waitpoint_enable(TEXT)
RETURNS VOID
AS :MODULE_PATHNAME, 'ts_debug_point_enable'
LANGUAGE C VOLATILE STRICT;
CREATE OR REPLACE FUNCTION debug_waitpoint_release(TEXT)
RETURNS VOID
AS :MODULE_PATHNAME, 'ts_debug_point_release'
LANGUAGE C VOLATILE STRICT;
CREATE OR REPLACE FUNCTION add_loopback_server(
server_name NAME,
host TEXT = 'localhost',
Expand Down
26 changes: 0 additions & 26 deletions tsl/test/isolation/expected/telemetry_iso.out

This file was deleted.

55 changes: 27 additions & 28 deletions tsl/test/isolation/specs/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -3,33 +3,9 @@ set(TEST_TEMPLATES_MODULE reorder_deadlock.spec.in
reorder_vs_insert_other_chunk.spec.in)

set(TEST_TEMPLATES_MODULE_DEBUG
reorder_vs_insert.spec.in
reorder_vs_select.spec.in
remote_create_chunk.spec.in
dist_su_copy_chunk.spec.in
dist_ha_chunk_drop.spec.in
dist_restore_point.spec.in
dist_cmd_exec.spec.in
cagg_drop_chunks_iso.spec.in
telemetry_iso.spec.in
compression_chunk_race.spec.in
compression_merge_race.spec.in
decompression_chunk_and_parallel_query.in
decompression_chunk_and_parallel_query_wo_idx.in)

# 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"
OR PG_VERSION VERSION_GREATER "13.3"
OR PG_VERSION VERSION_GREATER_EQUAL "14.0")
list(APPEND TEST_TEMPLATES_MODULE_DEBUG deadlock_recompress_chunk.spec.in)
endif()

if(PG_VERSION VERSION_GREATER_EQUAL "14.0")
list(APPEND TEST_TEMPLATES_MODULE_DEBUG freeze_chunk.spec.in
compression_dml_iso.spec)
endif()
reorder_vs_insert.spec.in reorder_vs_select.spec.in
dist_su_copy_chunk.spec.in dist_cmd_exec.spec.in
decompression_chunk_and_parallel_query.in)

list(
APPEND
Expand All @@ -44,7 +20,30 @@ list(

if(CMAKE_BUILD_TYPE MATCHES Debug)
list(APPEND TEST_TEMPLATES_MODULE ${TEST_TEMPLATES_MODULE_DEBUG})
list(APPEND TEST_FILES cagg_multi_dist_ht.spec)
list(
APPEND
TEST_FILES
cagg_drop_chunks_iso.spec
cagg_multi_dist_ht.spec
compression_chunk_race.spec
compression_merge_race.spec
decompression_chunk_and_parallel_query_wo_idx.spec
dist_ha_chunk_drop.spec
dist_ha_chunk_drop.spec
dist_restore_point.spec
remote_create_chunk.spec)
if(PG_VERSION VERSION_GREATER_EQUAL "14.0")
list(APPEND TEST_FILES freeze_chunk.spec compression_dml_iso.spec)
endif()
# 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"
Copy link
Member

Choose a reason for hiding this comment

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

We removed support for PG12 recenlty

Copy link
Contributor Author

Choose a reason for hiding this comment

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

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.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

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.

OR PG_VERSION VERSION_GREATER "13.3"
OR PG_VERSION VERSION_GREATER_EQUAL "14.0")
list(APPEND TEST_FILES deadlock_recompress_chunk.spec)
endif()

endif(CMAKE_BUILD_TYPE MATCHES Debug)

# need to generate MODULE name for the .spec files
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,12 +3,6 @@
# LICENSE-TIMESCALE for a copy of the license.

setup {
CREATE OR REPLACE FUNCTION debug_waitpoint_enable(TEXT) RETURNS VOID LANGUAGE C VOLATILE STRICT
AS '@TS_MODULE_PATHNAME@', 'ts_debug_point_enable';

CREATE OR REPLACE FUNCTION debug_waitpoint_release(TEXT) RETURNS VOID LANGUAGE C VOLATILE STRICT
AS '@TS_MODULE_PATHNAME@', 'ts_debug_point_release';

CREATE TABLE conditions("time" timestamptz, temp float);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,12 +7,6 @@
###

setup {
CREATE OR REPLACE FUNCTION debug_waitpoint_enable(TEXT) RETURNS VOID LANGUAGE C VOLATILE STRICT
AS '@TS_MODULE_PATHNAME@', 'ts_debug_point_enable';

CREATE OR REPLACE FUNCTION debug_waitpoint_release(TEXT) RETURNS VOID LANGUAGE C VOLATILE STRICT
AS '@TS_MODULE_PATHNAME@', 'ts_debug_point_release';

CREATE TABLE sensor_data (
time timestamptz not null,
sensor_id integer not null,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,13 +7,6 @@
###

setup {
CREATE OR REPLACE FUNCTION debug_waitpoint_enable(TEXT) RETURNS VOID LANGUAGE C VOLATILE STRICT
AS '@TS_MODULE_PATHNAME@', 'ts_debug_point_enable';

CREATE OR REPLACE FUNCTION debug_waitpoint_release(TEXT) RETURNS VOID LANGUAGE C VOLATILE STRICT
AS '@TS_MODULE_PATHNAME@', 'ts_debug_point_release';


-- Compressing a lot of chunks from a single hypertable in a single transaction can
-- cause deadlocks due to locking the compressed hypertable when creating compressed chunks.
-- Hence we compress them in their individual transactions, similar how the compression
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,12 +10,6 @@
###

setup {
CREATE OR REPLACE FUNCTION debug_waitpoint_enable(TEXT) RETURNS VOID LANGUAGE C VOLATILE STRICT
AS '@TS_MODULE_PATHNAME@', 'ts_debug_point_enable';

CREATE OR REPLACE FUNCTION debug_waitpoint_release(TEXT) RETURNS VOID LANGUAGE C VOLATILE STRICT
AS '@TS_MODULE_PATHNAME@', 'ts_debug_point_release';

CREATE TABLE sensor_data (
time timestamptz not null,
sensor_id integer not null,
Expand Down