Dear Hou,
Thanks for updating the patch. Comments for v12-0002.
01.
```
+ /* Free the existing invalid cache entries */
+ foreach_ptr(LogicalRepSequenceInfo, seqinfo, sequence_infos)
+ {
+ pfree(seqinfo->nspname);
+ pfree(seqinfo->seqname);
+ pfree(seqinfo);
+ }
```
According to the comment atop foreach_delete_current, we should not directly
pfree()
the iterator.
02.
```
+ /* Cache the information in a permanent memory context */
+ oldctx = MemoryContextSwitchTo(CacheMemoryContext);
```
Do you have a reason to use CacheMemoryContext instead of ApplyContext?
According to the readme, the context can be used for the limited purpose, like
catcache
and relcache. Not sure we can easily use it.
03.
I think seqinfo->found_on_pub must be set to false before doing
copy_sequences() again.
Otherwise, sequences dropped on the publisher cannot be detected as the missing
ones.
Or we may have to have another array to indicate it.
I found a below scenario.
There are 10 sequences on both instances, and sequencesync worker synchronizes
once.
Now two of them are dropped on the publisher. In the next iteration by the
worker,
it can find that only 8 sequeces are found on the publisher. Then it scans the
cache to check each found_on_pub in sequence_infos, but they were cached as
true.
Thus sequencesync worker cannot report anything for missing ones.
Best regards,
Hayato Kuroda
FUJITSU LIMITED
