Hello!

Some feedback for v33.

> else if (pg_strcasecmp(cmd, "REPACK") == 0)
>      cmdtype = PROGRESS_COMMAND_REPACK;
src/backend/utils/adt/pgstatfuncs.c:290

I think we need to add "CLUSTER" here too to avoid regression.

-----------

> ConditionVariablePrepareToSleep(&shared->cv);
>   for (;;)
>   {
>      bool      initialized;
>
>      SpinLockAcquire(&shared->mutex);
>      initialized = shared->initialized;
>      SpinLockRelease(&shared->mutex);
src/backend/commands/cluster.c:3663

I think we should check GetBackgroundWorkerPid for worker status, to
not wait forever in case of some issue with the worker.

-----------

> /* Error queue. */
> shm_mq       *error_mq;
src/backend/commands/cluster.c:210.

Not used anywhere.

-----------

>   finish_heap_swap(old_table_oid, new_table_oid,
>                is_system_catalog,
>                false,       /* swap_toast_by_content */
>                false, true, false,
>                frozenXid, cutoffMulti,
>                relpersistence);
src/backend/commands/cluster.c

I think we should add comments for other boolean parameters.

-----------

> elog(ERROR, "Incomplete insert info.");
> elog(ERROR, "Incomplete update info.");
src/backend/replication/pgoutput_repack/pgoutput_repack.c:118,132

Should be non-capitalized?

-----------

> # Copyright (c) 2022-2024, PostgreSQL Global Development Group
src/backend/replication/pgoutput_repack/meson.build

2022-2026

-----------

> int         newxcnt = 0;
src/backend/replication/logical/snapbuild.c:53

uint32 is better here.


Best regards,
Mikhail.


Reply via email to