On Mon, Aug 31, 2020 at 10:36:13AM +0530, Rahila wrote: > Now, the relreplident is being set in the transaction previous to > the one marking index as invalid for concurrent drop. Won't > it cause issues with relreplident cleared but index not dropped, > if drop index concurrently fails in the small window after > commit transaction in above snippet and before the start transaction above?
Argh. I missed your point that this stuff uses heap_inplace_update(), so the update of indisvalid flag is not transactional. The thing is that we won't be able to update the flag consistently with relreplident except if we switch index_set_state_flags() to use a transactional operation here. So, with the current state of affairs, it looks like we could just call SetRelationReplIdent() in the last transaction, after the transaction marking the index as dead has committed (see the top of index_set_state_flags() saying that this should be the last step of a transaction), but that leaves a window open as you say. On the other hand, it looks appealing to make index_set_state_flags() transactional. This would solve the consistency problem, and looking at the code scanning pg_index, I don't see a reason why we could not do that. What do you think? > To the existing partitioned table test, can we add a test to demonstrate > that relreplident is set to 'n' for even the individual partitions. Makes sense. It is still important to test the case where a partitioned table without leaves is correctly reset IMO. > Typo, s/updates are visible/updates visible Thanks. > - For DROP INDEX CONCURRENTLY, pg_class.relreplident of the parent > table is set in the last transaction doing the drop. It would be > tempting to reset the flag in the same transaction as the one marking > the index as invalid, but there could be a point in reindexing the > invalid index whose drop has failed, and this adds morecomplexity to > the patch. That's a point I studied, but it makes actually more sense to just reset the flag once the index is marked as invalid, similarly to indisclustered. Reindexing an invalid index can have value in some cases, but we would have much more problems with the relcache if we finish with two indexes marked as indreplident :( -- Michael
signature.asc
Description: PGP signature
