Re: Logical replication launcher did not automatically restart when got SIGKILL

Hi, The v1-0002 in [1] will call ReportBackgroundWorkerExit() which will send SIGUSR1 to 'bgw_notify_pid', but it may already exit in HandleChildCrash(), is this ok? [1] https://commitfest.postgresql.org/patch/5844/ -- Regards, ChangAo Chen

Re: Logical replication launcher did not automatically restart when got SIGKILL

Hi, Reset the PID in ResetBackgroundWorkerCrashTimes() may also works, but I'm not sure which is better. -- Regards, ChangAo Chen

Logical replication launcher did not automatically restart when got SIGKILL

Hi, hackers I found the $SUBJECT, the main reason is that RegisteredBgWorker::rw_pid has not been cleaned. Attach a patch to fix it. -- Regards, ChangAo Chen v1-0001-logical-replication-launcher-did-not-automaticall.patch Description: Binary data

Re: Small optimization with expanding dynamic hash table

Hi, The v3 patch attached. (Add some comment in commit msg) -- Regards, ChangAo Chen v3-0001-Small-optimization-with-expanding-dynamic-hash-ta.patch Description: Binary data

Re: Small optimization with expanding dynamic hash table

Hi, The v2 patch maybe more clear: We can calc bucket just by hashvalue & high_mask when expanding table because the if condition in calc_bucket() must be false. -- Regards, ChangAo Chen v2-0001-Small-optimization-with-expanding-dynamic-hash-ta.patch Description: Binary data

Re: Small optimization with expanding dynamic hash table

> One thing to note is that in this scenario, there is no safeguard if the hashvalue is 0x111 and new_bucket is 0x110. But the hash table is already corrupted if the hashvalue 0x111 in old_bucket 0x010, all hashvalue in old_bucket should have: hashvalue & low_mask == old_bucket's no. -- Regar

Re: Small optimization with expanding dynamic hash table

> Will this still work if new_bucket is not equal to hctl->low_mask + 1?Yes, for example: low_mask: 0x011, high_mask: 0x111, old_bucket: 0x010, new_bucket: 0x110 The old_bucket's hash value like 0x***010 or 0x***110, the later is in the old_bucket is because we didn't have new_bucket before,

Small optimization with expanding dynamic hash table

Hi, hackers If I understand correctly, we only need to check the specific bit to determine whether a hash element needs relocation: diff --git a/src/backend/utils/hash/dynahash.c b/src/backend/utils/hash/dynahash.c index 1ad155d446e..32fbae71995 100644 --- a/src/backend/utils/hash/dynahash.c

Re: Historic snapshot doesn't track txns committed in BUILDING_SNAPSHOT state

Hi, If I understand correctly, this may break the basic principle: The aim is to build a snapshot that behaves the same as a freshly taken MVCC snapshot would have at the time the XLogRecord was generated. -- Regards, ChangAo Chen -- Original -- From:

Re: Historic snapshot doesn't track txns committed in BUILDING_SNAPSHOT state

Hi, This change doesn't solve a bug. But I think it makes the code and comments more consistent. I currently have no idea about how to test it. -- Regards, ChangAo Chen

Re: Historic snapshot doesn't track txns committed in BUILDING_SNAPSHOT state

Hi, According to the comment above DecodeTXNNeedSkip(), transactions committed before SNAPBUILD_CONSISTENT state won't be decoded. It's important for initial table data synchronization because the table sync workers use the consistent snapshot to copy data so transactions before SNAPBUILD_CONSI

Re: Historic snapshot doesn't track txns committed in BUILDING_SNAPSHOT state

Hi, There are 3 threads currently. The latest patches are v4-0001 and v3-0002 in [2]. [1]https://www.postgresql.org/message-id/flat/tencent_6aaf072a7623a11a85c0b5fd290232467...@qq.com [2]https://www.postgresql.org/message-id/flat/tencent_8dec9842690a9b6afd52d4659ef0700e9...@qq.com [3]https://

Re: Fix a wrong errmsg in AlterRole()

Hi, I modified the patch according to Tom's suggestion. -- Regards, ChangAo Chen v2-0001-Fix-a-wrong-errdetail-in-AlterRole.patch Description: Binary data

Fix a wrong errmsg in AlterRole()

Hi, ``` postgres=# create group g1; CREATE ROLE postgres=# create role r1 in group g1; CREATE ROLE postgres=# set session authorization r1; SET postgres=> alter group g1 drop user r1; ERROR:  permission denied to alter role DETAIL:  Only roles with the ADMIN option on role "g1" may add members. `

Re: Fix a wrong comment in load_file()

> The saved hooks are not here to readjust the stack based on a reload, > just to make sure that the existing paths loaded are all taken.  I > would just remove the "in case of unload" part for the last three, and > "unload" for the first one.  Thoughts? LGTM. -- Regards, ChangAo Chen

Fix a wrong comment in load_file()

Hi, The comment in load_file(): /* If the same shlib has previously been loaded, unload and reload it. */ But we currently don't support to unload a shlib. Here is a small patch to fix it. -- Regards, ChangAo Chen 0001-Fix-a-wrong-comment-in-load_file.patch Description: Binary data

Re: Reduce one comparison in binaryheap's sift down

Agree, new version patch is attached. -- Regards, ChangAo Chen -- Original -- From: "Nathan Bossart"

Reduce one comparison in binaryheap's sift down

Hi, I think we can reduce one comparison in binaryheap's sift down, right? Here is a patch to fix it. -- Regards, ChangAo Chen v1-0001-Reduce-one-comparison-in-binaryheap-s-sift-down.patch Description: Binary data

Re: Historic snapshot doesn't track txns committed in BUILDING_SNAPSHOT state

Hi, - IIUC your "fast forward" concern is not related to this particular thread but you - think it's already an issue on the master branch (outside of the BUILDING_SNAPSHOT - handling we are discussing here), is that correct? (that's also what your coding - changes makes me think of). If so, I

Re: Historic snapshot doesn't track txns committed in BUILDING_SNAPSHOT state

Hi, - I re-read your comments in [0] and it looks like you've concern about - the 2 "if" I'm proposing above and the fast forward handling. Is that the case - or is your fast forward concern unrelated to my proposals? In your proposals, we will just return when fast forward. But I think we need

Re: Historic snapshot doesn't track txns committed in BUILDING_SNAPSHOT state

Hi, I refactor the code and fix the git apply warning according to [1]. Here are the new version patches. -- Regards, ChangAo Chen [1] https://www.postgresql.org/message-id/Zrmh7X8jYCbFYXjH%40ip-10-97-1-34.eu-west-3.compute.internal v3-0002-Add-test-case-snapshot_build-for-test_decoding.pa

Re: Historic snapshot doesn't track txns committed in BUILDING_SNAPSHOT state

Hi, Thanks for the comments! - I think it's fine to skip during fast forward as we are not generating logical - changes. It's done that way in master, in your proposal and in my "if" proposals. - Note that my proposals related to the if conditions are for heap2_decode and - heap_decode (not xac

Re: Historic snapshot doesn't track txns committed in BUILDING_SNAPSHOT state

Hi, 4, 5 === > if (SnapBuildCurrentState(builder) < SNAPBUILD_BUILDING_SNAPSHOT || >     (SnapBuildCurrentState(builder) == SNAPBUILD_BUILDING_SNAPSHOT && info != XLOG_HEAP_INPLACE) || >     ctx->fast_forward) >     return; I think during fast forward, we also need handle the xlog that mark

Re: Historic snapshot doesn't track txns committed in BUILDING_SNAPSHOT state

Hi, Thanks for the comments! Here are the new version patches, I think it will be more clear. -- Regards, ChangAo Chen v3-0001-Track-transactions-committed-in-BUILDING_SNAPSHOT.patch Description: Binary data v3-0002-Add-test-case-snapshot_build-for-test_decoding.patch Description: Binary d

Re: Historic snapshot doesn't track txns committed in BUILDING_SNAPSHOT state

Hi, Thanks for pointing it out! Here are the new version patches with a test case. -- Regards, ChangAo Chen -- Original -- From: "Bertrand

Re: Redundant syscache access in get_rel_sync_entry()

-- Original -- From: "Ashutosh Bapat"

Re: Fix a comment error in logicalrep_write_typ()

Thank you for review! The commitfest link for tracking: https://commitfest.postgresql.org/49/5121/ -- Regards, ChangAo Chen

Fix a comment error in logicalrep_write_typ()

Hi, -       /* use Oid as relation identifier */ +       /* use Oid as type identifier */         pq_sendint32(out, typoid); I think it must be "type identifier" rather than "relation identifier". -- Regards, ChangAo Chen 0001-Fix-a-comment-error-in-logicalrep_write_typ.patch Description: B

Redundant syscache access in get_rel_sync_entry()

Hi, in func get_rel_sync_entry() we access the same tuple in pg_class three times:     Oid         schemaId = get_rel_namespace(relid);     bool am_partition = get_rel_relispartition(relid);     char relkind = get_rel_relkind(relid); Why not just merge into one?

Re: Historic snapshot doesn't track txns committed in BUILDING_SNAPSHOT state

Thank you for reply!I am trying to fix it. This patch (pass check-world) will track txns committed in BUILDING_SNAPSHOT state and can fix this bug. -- Regards, ChangAo Chen v1-0001-Track-transactions-committed-in-BUILDING_SNAPSHOT.patch Description: Binary data

Re: Format the code in xact_decode

Thank you for reply! I have new a patch in commitfest:Format the code in xact_decode (postgresql.org) -- Regards, ChangAo Chen

Historic snapshot doesn't track txns committed in BUILDING_SNAPSHOT state

Hello hackers, I found that we currently don't track txns committed in BUILDING_SNAPSHOT state because of the code in xact_decode(): /* * If the snapshot isn't yet fully built, we cannot decode anything, so * bail out. */ if (SnapBuildCurrentState(builder)

Format the code in xact_decode

Hello hackers, just as the title says: 1. Remove redundant parentheses. 2. Adjust the position of the break statement. -- Regards, ChangAo Chen 0001-Format-the-code-in-xact_decode.patch Description: Binary data

Re:Bug report and fix about building historic snapshot

> This patch may be better, which only track catalog modified transactions. Can anyone help review this patch? Thanks. -- Regards, ChangAo Chen

Re:Bug report and fix about building historic snapshot

This patch may be better, which only track catalog modified transactions. -- Regards, ChangAo Chen -- Original -- From: "cc

Bug report and fix about building historic snapshot

Hello, I find a bug in building historic snapshot and the steps to reproduce are as follows: Prepare: (pub)create table t1 (id int primary key); (pub)insert into t1 values (1); (pub)create publication pub for table t1; (sub)create table t1 (id int primary key); Reproduce: (pub)beg

[pg16]Question about building snapshot in logical decoding

If txn1 begin after SNAPBUILD_BUILDING_SNAPSHOT and commit before SNAPBUILD_FULL_SNAPSHOT(so txn1 will not be processed by DecodeCommit), and txn2 begin after SNAPBUILD_FULL_SNAPSHOT and commit after SNAPBUILD_CONSISTENT(so txn2 will be replayed), how to ensure that txn2 could see the changes made