From 2bf1472c8e56d13d88169ed6c1c38cede98c8c23 Mon Sep 17 00:00:00 2001 From: Gokhan Gulbiz Date: Thu, 26 Oct 2023 14:54:58 +0300 Subject: [PATCH 1/3] Move GHA environment variables to workflow file (#7275) Since GHA does not interpolate env variables in a matrix context, This PR defines them in a separate job and uses them in other jobs. --- .github/workflows/build_and_test.yml | 131 +++++++++++------- .../workflows/packaging-test-pipelines.yml | 8 +- 2 files changed, 85 insertions(+), 54 deletions(-) diff --git a/.github/workflows/build_and_test.yml b/.github/workflows/build_and_test.yml index 5944c38db..1f22ff034 100644 --- a/.github/workflows/build_and_test.yml +++ b/.github/workflows/build_and_test.yml @@ -13,10 +13,33 @@ on: pull_request: types: [opened, reopened,synchronize] jobs: + # Since GHA does not interpolate env varibles in matrix context, we need to + # define them in a separate job and use them in other jobs. + params: + runs-on: ubuntu-latest + name: Initialize parameters + outputs: + build_image_name: "citus/extbuilder" + test_image_name: "citus/exttester" + citusupgrade_image_name: "citus/citusupgradetester" + fail_test_image_name: "citus/failtester" + pgupgrade_image_name: "citus/pgupgradetester" + style_checker_image_name: "citus/stylechecker" + style_checker_tools_version: "0.8.18" + image_suffix: "-v9d71045" + pg14_version: "14.9" + pg15_version: "15.4" + pg16_version: "16.0" + upgrade_pg_versions: "14.9-15.4-16.0" + steps: + # Since GHA jobs needs at least one step we use a noop step here. + - name: Set up parameters + run: echo 'noop' check-sql-snapshots: + needs: params runs-on: ubuntu-20.04 container: - image: ${{ vars.build_image_name }}:latest + image: ${{ needs.params.outputs.build_image_name }}:latest options: --user root steps: - uses: actions/checkout@v3.5.0 @@ -25,9 +48,10 @@ jobs: git config --global --add safe.directory ${GITHUB_WORKSPACE} ci/check_sql_snapshots.sh check-style: + needs: params runs-on: ubuntu-20.04 container: - image: ${{ vars.style_checker_image_name }}:${{ vars.style_checker_tools_version }}${{ vars.image_suffix }} + image: ${{ needs.params.outputs.style_checker_image_name }}:${{ needs.params.outputs.style_checker_tools_version }}${{ needs.params.outputs.image_suffix }} steps: - name: Check Snapshots run: | @@ -68,18 +92,19 @@ jobs: - name: Check for missing downgrade scripts run: ci/check_migration_files.sh build: + needs: params name: Build for PG ${{ matrix.pg_version}} strategy: fail-fast: false matrix: image_name: - - ${{ vars.build_image_name }} + - ${{ needs.params.outputs.build_image_name }} image_suffix: - - ${{ vars.image_suffix}} + - ${{ needs.params.outputs.image_suffix}} pg_version: - - ${{ vars.pg14_version }} - - ${{ vars.pg15_version }} - - ${{ vars.pg16_version }} + - ${{ needs.params.outputs.pg14_version }} + - ${{ needs.params.outputs.pg15_version }} + - ${{ needs.params.outputs.pg16_version }} runs-on: ubuntu-20.04 container: image: "${{ matrix.image_name }}:${{ matrix.pg_version }}${{ matrix.image_suffix }}" @@ -106,11 +131,11 @@ jobs: suite: - regress image_name: - - ${{ vars.test_image_name }} + - ${{ needs.params.outputs.test_image_name }} pg_version: - - ${{ vars.pg14_version }} - - ${{ vars.pg15_version }} - - ${{ vars.pg16_version }} + - ${{ needs.params.outputs.pg14_version }} + - ${{ needs.params.outputs.pg15_version }} + - ${{ needs.params.outputs.pg16_version }} make: - check-split - check-multi @@ -129,69 +154,70 @@ jobs: - check-enterprise-isolation-logicalrep-3 include: - make: check-failure - pg_version: ${{ vars.pg14_version }} + pg_version: ${{ needs.params.outputs.pg14_version }} suite: regress - image_name: ${{ vars.fail_test_image_name }} + image_name: ${{ needs.params.outputs.fail_test_image_name }} - make: check-failure - pg_version: ${{ vars.pg15_version }} + pg_version: ${{ needs.params.outputs.pg15_version }} suite: regress - image_name: ${{ vars.fail_test_image_name }} + image_name: ${{ needs.params.outputs.fail_test_image_name }} - make: check-failure - pg_version: ${{ vars.pg16_version }} + pg_version: ${{ needs.params.outputs.pg16_version }} suite: regress - image_name: ${{ vars.fail_test_image_name }} + image_name: ${{ needs.params.outputs.fail_test_image_name }} - make: check-enterprise-failure - pg_version: ${{ vars.pg14_version }} + pg_version: ${{ needs.params.outputs.pg14_version }} suite: regress - image_name: ${{ vars.fail_test_image_name }} + image_name: ${{ needs.params.outputs.fail_test_image_name }} - make: check-enterprise-failure - pg_version: ${{ vars.pg15_version }} + pg_version: ${{ needs.params.outputs.pg15_version }} suite: regress - image_name: ${{ vars.fail_test_image_name }} + image_name: ${{ needs.params.outputs.fail_test_image_name }} - make: check-enterprise-failure - pg_version: ${{ vars.pg16_version }} + pg_version: ${{ needs.params.outputs.pg16_version }} suite: regress - image_name: ${{ vars.fail_test_image_name }} + image_name: ${{ needs.params.outputs.fail_test_image_name }} - make: check-pytest - pg_version: ${{ vars.pg14_version }} + pg_version: ${{ needs.params.outputs.pg14_version }} suite: regress - image_name: ${{ vars.fail_test_image_name }} + image_name: ${{ needs.params.outputs.fail_test_image_name }} - make: check-pytest - pg_version: ${{ vars.pg15_version }} + pg_version: ${{ needs.params.outputs.pg15_version }} suite: regress - image_name: ${{ vars.fail_test_image_name }} + image_name: ${{ needs.params.outputs.fail_test_image_name }} - make: check-pytest - pg_version: ${{ vars.pg16_version }} + pg_version: ${{ needs.params.outputs.pg16_version }} suite: regress - image_name: ${{ vars.fail_test_image_name }} + image_name: ${{ needs.params.outputs.fail_test_image_name }} - make: installcheck suite: cdc - image_name: ${{ vars.test_image_name }} - pg_version: ${{ vars.pg15_version }} + image_name: ${{ needs.params.outputs.test_image_name }} + pg_version: ${{ needs.params.outputs.pg15_version }} - make: installcheck suite: cdc - image_name: ${{ vars.test_image_name }} - pg_version: ${{ vars.pg16_version }} + image_name: ${{ needs.params.outputs.test_image_name }} + pg_version: ${{ needs.params.outputs.pg16_version }} - make: check-query-generator - pg_version: ${{ vars.pg14_version }} + pg_version: ${{ needs.params.outputs.pg14_version }} suite: regress - image_name: ${{ vars.fail_test_image_name }} + image_name: ${{ needs.params.outputs.fail_test_image_name }} - make: check-query-generator - pg_version: ${{ vars.pg15_version }} + pg_version: ${{ needs.params.outputs.pg15_version }} suite: regress - image_name: ${{ vars.fail_test_image_name }} + image_name: ${{ needs.params.outputs.fail_test_image_name }} - make: check-query-generator - pg_version: ${{ vars.pg16_version }} + pg_version: ${{ needs.params.outputs.pg16_version }} suite: regress - image_name: ${{ vars.fail_test_image_name }} + image_name: ${{ needs.params.outputs.fail_test_image_name }} runs-on: ubuntu-20.04 container: - image: "${{ matrix.image_name }}:${{ matrix.pg_version }}${{ vars.image_suffix }}" + image: "${{ matrix.image_name }}:${{ matrix.pg_version }}${{ needs.params.outputs.image_suffix }}" options: --user root --dns=8.8.8.8 # Due to Github creates a default network for each job, we need to use # --dns= to have similar DNS settings as our other CI systems or local # machines. Otherwise, we may see different results. needs: + - params - build steps: - uses: actions/checkout@v3.5.0 @@ -212,19 +238,20 @@ jobs: name: PG${{ matrix.pg_version }} - check-arbitrary-configs-${{ matrix.parallel }} runs-on: ["self-hosted", "1ES.Pool=1es-gha-citusdata-pool"] container: - image: "${{ matrix.image_name }}:${{ matrix.pg_version }}${{ vars.image_suffix }}" + image: "${{ matrix.image_name }}:${{ matrix.pg_version }}${{ needs.params.outputs.image_suffix }}" options: --user root needs: + - params - build strategy: fail-fast: false matrix: image_name: - - ${{ vars.fail_test_image_name }} + - ${{ needs.params.outputs.fail_test_image_name }} pg_version: - - ${{ vars.pg14_version }} - - ${{ vars.pg15_version }} - - ${{ vars.pg16_version }} + - ${{ needs.params.outputs.pg14_version }} + - ${{ needs.params.outputs.pg15_version }} + - ${{ needs.params.outputs.pg16_version }} parallel: [0,1,2,3,4,5] # workaround for running 6 parallel jobs steps: - uses: actions/checkout@v3.5.0 @@ -258,9 +285,10 @@ jobs: name: PG${{ matrix.old_pg_major }}-PG${{ matrix.new_pg_major }} - check-pg-upgrade runs-on: ubuntu-20.04 container: - image: "${{ vars.pgupgrade_image_name }}:${{ vars.upgrade_pg_versions }}${{ vars.image_suffix }}" + image: "${{ needs.params.outputs.pgupgrade_image_name }}:${{ needs.params.outputs.upgrade_pg_versions }}${{ needs.params.outputs.image_suffix }}" options: --user root needs: + - params - build strategy: fail-fast: false @@ -305,12 +333,13 @@ jobs: flags: ${{ env.old_pg_major }}_${{ env.new_pg_major }}_upgrade codecov_token: ${{ secrets.CODECOV_TOKEN }} test-citus-upgrade: - name: PG${{ vars.pg14_version }} - check-citus-upgrade + name: PG${{ needs.params.outputs.pg14_version }} - check-citus-upgrade runs-on: ubuntu-20.04 container: - image: "${{ vars.citusupgrade_image_name }}:${{ vars.pg14_version }}${{ vars.image_suffix }}" + image: "${{ needs.params.outputs.citusupgrade_image_name }}:${{ needs.params.outputs.pg14_version }}${{ needs.params.outputs.image_suffix }}" options: --user root needs: + - params - build steps: - uses: actions/checkout@v3.5.0 @@ -354,8 +383,9 @@ jobs: CC_TEST_REPORTER_ID: ${{ secrets.CC_TEST_REPORTER_ID }} runs-on: ubuntu-20.04 container: - image: ${{ vars.test_image_name }}:${{ vars.pg16_version }}${{ vars.image_suffix }} + image: ${{ needs.params.outputs.test_image_name }}:${{ needs.params.outputs.pg16_version }}${{ needs.params.outputs.image_suffix }} needs: + - params - test-citus - test-arbitrary-configs - test-citus-upgrade @@ -448,11 +478,12 @@ jobs: name: Test flakyness runs-on: ubuntu-20.04 container: - image: ${{ vars.fail_test_image_name }}:${{ vars.pg16_version }}${{ vars.image_suffix }} + image: ${{ needs.params.outputs.fail_test_image_name }}:${{ needs.params.outputs.pg16_version }}${{ needs.params.outputs.image_suffix }} options: --user root env: runs: 8 needs: + - params - build - test-flakyness-pre - prepare_parallelization_matrix_32 diff --git a/.github/workflows/packaging-test-pipelines.yml b/.github/workflows/packaging-test-pipelines.yml index 9d3fb81be..0fb4b7092 100644 --- a/.github/workflows/packaging-test-pipelines.yml +++ b/.github/workflows/packaging-test-pipelines.yml @@ -24,14 +24,14 @@ jobs: - name: Get Postgres Versions id: get-postgres-versions run: | - # Postgres versions are stored in .circleci/config.yml file in "build-[pg-version] format. Below command - # extracts the versions and get the unique values. - pg_versions=`grep -Eo 'build-[[:digit:]]{2}' .circleci/config.yml|sed -e "s/^build-//"|sort|uniq|tr '\n' ','| head -c -1` + # Postgres versions are stored in .github/workflows/build_and_test.yml file in "pg[pg-version]_version" + # format. Below command extracts the versions and get the unique values. + pg_versions=$(cat .github/workflows/build_and_test.yml | grep -oE 'pg[0-9]+_version: "[0-9.]+"' | sed -E 's/pg([0-9]+)_version: "([0-9.]+)"/\1/g' | sort | uniq | tr '\n', ',') pg_versions_array="[ ${pg_versions} ]" echo "Supported PG Versions: ${pg_versions_array}" # Below line is needed to set the output variable to be used in the next job echo "pg_versions=${pg_versions_array}" >> $GITHUB_OUTPUT - + shell: bash rpm_build_tests: name: rpm_build_tests needs: get_postgres_versions_from_file From f9218d9780a88356b7f6861bb1cc07781c1df808 Mon Sep 17 00:00:00 2001 From: Benjamin O Date: Fri, 27 Oct 2023 10:42:55 -0400 Subject: [PATCH 2/3] Support replacing IPv6 Loopback in `normalize.sed` (#7269) I had a test failure issue due to my machine using the IPv6 loopback address. This change to the `normalize.sed` solves that issue. --- src/test/regress/bin/normalize.sed | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/test/regress/bin/normalize.sed b/src/test/regress/bin/normalize.sed index efa9e310f..1d293e964 100644 --- a/src/test/regress/bin/normalize.sed +++ b/src/test/regress/bin/normalize.sed @@ -222,7 +222,7 @@ s/(CONTEXT: PL\/pgSQL function .* line )([0-9]+)/\1XX/g s/^(PL\/pgSQL function .* line) [0-9]+ (.*)/\1 XX \2/g # normalize a test difference in multi_move_mx -s/ connection to server at "\w+" \(127\.0\.0\.1\), port [0-9]+ failed://g +s/ connection to server at "\w+" (\(127\.0\.0\.1\)|\(::1\)), port [0-9]+ failed://g # normalize differences in tablespace of new index s/pg14\.idx.*/pg14\.xxxxx/g From d0b093c975c8b3056f9e35c81903e7cfa05644d2 Mon Sep 17 00:00:00 2001 From: Nils Dijk Date: Fri, 27 Oct 2023 16:57:51 +0200 Subject: [PATCH 3/3] automatically add a breakpoint that breaks on postgres errors (#7279) When debugging postgres it is quite hard to get to the source for `errfinish` in `elog.c`. Instead of relying on the developer to set a breakpoint in the `elog.c` file for `errfinish` for `elevel == ERROR`, this change adds the breakpoint to `.gdbinit`. This makes sure that whenever a debugger is attached to a postgres backend it will break on postgres errors. When attaching the debugger a small banner is printed that explains how to disable the breakpoint. --- .devcontainer/.gdbinit | 28 ++++++++++++++++++++++++++++ 1 file changed, 28 insertions(+) diff --git a/.devcontainer/.gdbinit b/.devcontainer/.gdbinit index 9c710923f..9d544512b 100644 --- a/.devcontainer/.gdbinit +++ b/.devcontainer/.gdbinit @@ -3,3 +3,31 @@ # actually also works when debugging with vscode. Providing nice tools # to understand the internal datastructures we are working with. source /root/gdbpg.py + +# when debugging postgres it is convenient to _always_ have a breakpoint +# trigger when an error is logged. Because .gdbinit is sourced before gdb +# is fully attached and has the sources loaded. To make sure the breakpoint +# is added when the library is loaded we temporary set the breakpoint pending +# to on. After we have added out breakpoint we revert back to the default +# configuration for breakpoint pending. +# The breakpoint is hard to read, but at entry of the function we don't have +# the level loaded in elevel. Instead we hardcode the location where the +# level of the current error is stored. Also gdb doesn't understand the +# ERROR symbol so we hardcode this to the value of ERROR. It is very unlikely +# this value will ever change in postgres, but if it does we might need to +# find a way to conditionally load the correct breakpoint. +set breakpoint pending on +break elog.c:errfinish if errordata[errordata_stack_depth].elevel == 21 +set breakpoint pending auto + +echo \n +echo ----------------------------------------------------------------------------------\n +echo when attaching to a postgres backend a breakpoint will be set on elog.c:errfinish \n +echo it will only break on errors being raised in postgres \n +echo \n +echo to disable this breakpoint from vscode run `-exec disable 1` in the debug console \n +echo this assumes it's the first breakpoint loaded as it is loaded from .gdbinit \n +echo this can be verified with `-exec info break`, enabling can be done with \n +echo `-exec enable 1` \n +echo ----------------------------------------------------------------------------------\n +echo \n