Chao Li <[email protected]> wrote: > I have not reviewed the repack-related patches before. Recently, I started > trying to understand how repack works and trace through the code.
Thanks, I appreciate that! > While tracing start_repack_decoding_worker(), I noticed something suspicious: > > ``` > seg = dsm_create(size, 0); > shared = (DecodingWorkerShared *) dsm_segment_address(seg); > shared->lsn_upto = InvalidXLogRecPtr; > shared->done = false; > SharedFileSetInit(&shared->sfs, seg); > shared->last_exported = -1; > SpinLockInit(&shared->mutex); > shared->dbid = MyDatabaseId; > ``` > > Here, the code creates a shared-memory segment and lets “shared" point to > that memory. It then initializes some fields of “shared". However, later code > reads shared->initialized, but this field was not initialized: The problem was noticed earlier this week and I already posted a fix [1]. > For the fix, since start_repack_decoding_worker() is not on a hot path, I > think it is fine to zero the whole shared struct explicitly, and then > initialize the non-zero fields afterwards. Although not strongly, I prefer setting individual fields explicitly. [1] https://www.postgresql.org/message-id/182883.1776073323%40localhost -- Antonin Houska Web: https://www.cybertec-postgresql.com
