Skip to content

Bump PG versions to 17.6, 16.10, 15.14 #8142

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.

Sign up for GitHub

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

Merged
naisila merged 4 commits into from
Aug 25, 2025
Merged

Bump PG versions to 17.6, 16.10, 15.14 #8142

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
61c7745
Bump to pg 17.6, 16.10, 15.14
naisila Aug 20, 2025
b5794df
Try release version
naisila Aug 22, 2025
f6a8235
Fix HaveRegisteredOrActiveSnapshot() crash
naisila Aug 22, 2025
c6c80de
Another snapshot assertion crash
naisila Aug 22, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 4 additions & 4 deletions .devcontainer/Dockerfile
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,7 @@ USER citus

# build postgres versions separately for effective parrallelism and caching of already built versions when changing only certain versions
FROM base AS pg15
RUN MAKEFLAGS="-j $(nproc)" pgenv build 15.13
RUN MAKEFLAGS="-j $(nproc)" pgenv build 15.14
RUN rm .pgenv/src/*.tar*
RUN make -C .pgenv/src/postgresql-*/ clean
RUN make -C .pgenv/src/postgresql-*/src/include install
Expand All @@ -85,7 +85,7 @@ RUN cp -r .pgenv/src .pgenv/pgsql-* .pgenv/config .pgenv-staging/
RUN rm .pgenv-staging/config/default.conf

FROM base AS pg16
RUN MAKEFLAGS="-j $(nproc)" pgenv build 16.9
RUN MAKEFLAGS="-j $(nproc)" pgenv build 16.10
RUN rm .pgenv/src/*.tar*
RUN make -C .pgenv/src/postgresql-*/ clean
RUN make -C .pgenv/src/postgresql-*/src/include install
Expand All @@ -97,7 +97,7 @@ RUN cp -r .pgenv/src .pgenv/pgsql-* .pgenv/config .pgenv-staging/
RUN rm .pgenv-staging/config/default.conf

FROM base AS pg17
RUN MAKEFLAGS="-j $(nproc)" pgenv build 17.5
RUN MAKEFLAGS="-j $(nproc)" pgenv build 17.6
RUN rm .pgenv/src/*.tar*
RUN make -C .pgenv/src/postgresql-*/ clean
RUN make -C .pgenv/src/postgresql-*/src/include install
Expand Down Expand Up @@ -216,7 +216,7 @@ COPY --chown=citus:citus .psqlrc .
RUN sudo chown --from=root:root citus:citus -R ~

# sets default pg version
RUN pgenv switch 17.5
RUN pgenv switch 17.6

# make connecting to the coordinator easy
ENV PGPORT=9700
12 changes: 6 additions & 6 deletions .github/workflows/build_and_test.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,12 +31,12 @@ jobs:
pgupgrade_image_name: "ghcr.io/citusdata/pgupgradetester"
style_checker_image_name: "ghcr.io/citusdata/stylechecker"
style_checker_tools_version: "0.8.18"
sql_snapshot_pg_version: "17.5"
image_suffix: "-vb17c33b"
pg15_version: '{ "major": "15", "full": "15.13" }'
pg16_version: '{ "major": "16", "full": "16.9" }'
pg17_version: '{ "major": "17", "full": "17.5" }'
upgrade_pg_versions: "15.13-16.9-17.5"
sql_snapshot_pg_version: "17.6"
image_suffix: "-v4df94a0"
pg15_version: '{ "major": "15", "full": "15.14" }'
pg16_version: '{ "major": "16", "full": "16.10" }'
pg17_version: '{ "major": "17", "full": "17.6" }'
upgrade_pg_versions: "15.14-16.10-17.6"
steps:
# Since GHA jobs need at least one step we use a noop step here.
- name: Set up parameters
Expand Down
3 changes: 2 additions & 1 deletion src/backend/columnar/columnar_metadata.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -659,8 +659,9 @@ SaveStripeSkipList(RelFileLocator relfilelocator, uint64 stripe,
nulls[Anum_columnar_chunk_minimum_value - 1] = true;
nulls[Anum_columnar_chunk_maximum_value - 1] = true;
}

PushActiveSnapshot(GetTransactionSnapshot());
Copy link
Copy Markdown
Contributor

@ivan-v-kush ivan-v-kush Oct 14, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

@naisila could you please tell why PushActiveSnapshot/PopActiveSnapshot are inside the loop rather than outside
Is it possible to do it this way to save resources?

	PushActiveSnapshot(GetTransactionSnapshot());
	for (columnIndex = 0; columnIndex < columnCount; columnIndex++)
	{
		for (chunkIndex = 0; chunkIndex < chunkList->chunkCount; chunkIndex++)
		{
			....
			InsertTupleAndEnforceConstraints(modifyState, values, nulls);
			...
		}
		...
	}
	PopActiveSnapshot();

your code:

	for (columnIndex = 0; columnIndex < columnCount; columnIndex++)
	{
		for (chunkIndex = 0; chunkIndex < chunkList->chunkCount; chunkIndex++)
		{
			...
			PushActiveSnapshot(GetTransactionSnapshot());

			InsertTupleAndEnforceConstraints(modifyState, values, nulls);

			PopActiveSnapshot();
			...
		}
		...
	}

Thank you!

InsertTupleAndEnforceConstraints(modifyState, values, nulls);
PopActiveSnapshot();
}
}

Expand Down
4 changes: 3 additions & 1 deletion src/backend/distributed/metadata/node_metadata.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3288,7 +3288,9 @@ InsertNodeRow(int nodeid, char *nodeName, int32 nodePort, NodeMetadata *nodeMeta
Int32GetDatum(nodeMetadata->nodeprimarynodeid);
HeapTuple heapTuple = heap_form_tuple(tupleDescriptor, values, isNulls);

CATALOG_INSERT_WITH_SNAPSHOT(pgDistNode, heapTuple);
PushActiveSnapshot(GetTransactionSnapshot());
Copy link
Copy Markdown
Contributor

@m3hm3t m3hm3t Aug 25, 2025

Choose a reason for hiding this comment

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

nitpick:

we may reduce code duplication by similar like

/* maybe under a utils file */
static inline void
CatalogInsertWithTxnSnapshot(Relation rel, HeapTuple tup)
{
    PushActiveSnapshot(GetTransactionSnapshot());
    CatalogTupleInsert(rel, tup);
    PopActiveSnapshot();
}

or more complex but more generic and can be used later as well

#define WITH_TXN_SNAPSHOT(...)                        \
    do {                                              \
        PushActiveSnapshot(GetTransactionSnapshot()); \
        __VA_ARGS__;                                  \
        PopActiveSnapshot();                          \
    } while (0)

WITH_TXN_SNAPSHOT(
    InsertTupleAndEnforceConstraints(modifyState, values, nulls)
);

WITH_TXN_SNAPSHOT(
    CatalogTupleInsert(pgDistTransaction, heapTuple)
);

Copy link
Copy Markdown
Contributor Author

@naisila naisila Aug 25, 2025

Choose a reason for hiding this comment

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

Prefer to go with Postgres approach postgres/postgres@706054b#diff-d373fb790605aded212c2e3a0fbe8c015d110077ff46c07d0a6d6647a2d7e928

CatalogTupleInsert(pgDistNode, heapTuple);
PopActiveSnapshot();

CitusInvalidateRelcacheByRelid(DistNodeRelationId());

Expand Down
4 changes: 3 additions & 1 deletion src/backend/distributed/transaction/transaction_recovery.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -106,7 +106,9 @@ LogTransactionRecord(int32 groupId, char *transactionName, FullTransactionId out
HeapTuple heapTuple = heap_form_tuple(tupleDescriptor, values, isNulls);

/* insert new tuple */
CATALOG_INSERT_WITH_SNAPSHOT(pgDistTransaction, heapTuple);
PushActiveSnapshot(GetTransactionSnapshot());
CatalogTupleInsert(pgDistTransaction, heapTuple);
PopActiveSnapshot();

CommandCounterIncrement();

Expand Down
19 changes: 0 additions & 19 deletions src/include/pg_version_compat.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,10 +13,6 @@

#include "pg_version_constants.h"

/* we need these for PG-18’s PushActiveSnapshot/PopActiveSnapshot APIs */
#include "access/xact.h"
#include "utils/snapmgr.h"

#if PG_VERSION_NUM >= PG_VERSION_18
#define create_foreignscan_path_compat(a, b, c, d, e, f, g, h, i, j, k) \
create_foreignscan_path( \
Expand All @@ -40,14 +36,6 @@
/* PG-18 unified row-compare operator codes under COMPARE_* */
#define ROWCOMPARE_NE COMPARE_NE

#define CATALOG_INSERT_WITH_SNAPSHOT(rel, tup) \
do { \
Snapshot __snap = GetTransactionSnapshot(); \
PushActiveSnapshot(__snap); \
CatalogTupleInsert((rel), (tup)); \
PopActiveSnapshot(); \
} while (0)

#elif PG_VERSION_NUM >= PG_VERSION_17
#define create_foreignscan_path_compat(a, b, c, d, e, f, g, h, i, j, k) \
create_foreignscan_path( \
Expand All @@ -56,9 +44,6 @@
(g), (h), (i), (j), (k) \
)

/* no-op wrapper on older PGs */
#define CATALOG_INSERT_WITH_SNAPSHOT(rel, tup) \
CatalogTupleInsert((rel), (tup))
#endif

#if PG_VERSION_NUM >= PG_VERSION_17
Expand Down Expand Up @@ -469,10 +454,6 @@ getStxstattarget_compat(HeapTuple tup)
k) create_foreignscan_path(a, b, c, d, e, f, g, h, \
i, k)

/* no-op wrapper on older PGs */
#define CATALOG_INSERT_WITH_SNAPSHOT(rel, tup) \
CatalogTupleInsert((rel), (tup))

#define getProcNo_compat(a) (a->pgprocno)
#define getLxid_compat(a) (a->lxid)

Expand Down