On Sat, Apr 4, 2015 at 3:10 PM, Jim Nasby <jim.na...@bluetreble.com> wrote:
> On 4/3/15 12:59 AM, Sawada Masahiko wrote: > >> + case HEAPTUPLE_LIVE: >> + case HEAPTUPLE_RECENTLY_DEAD: >> + case HEAPTUPLE_INSERT_IN_PROGRESS: >> + case HEAPTUPLE_DELETE_IN_PROGRESS: >> + if >> (heap_prepare_freeze_tuple(tuple.t_data, >> freezelimit, >> + >> mxactcutoff, &frozen[nfrozen])) >> + frozen[nfrozen++].offset >> = offnum; >> + break; >> > > This doesn't seem safe enough to me. Can't there be tuples that are still > new enough that they can't be frozen, and are still live? Yep. I've set a table to read only while it contained unfreezable tuples, and the tuples remain unfrozen yet the read-only action claims to have succeeded. > Somewhat related... instead of forcing the freeze to happen synchronously, > can't we set this up so a table is in one of three states? Read/Write, Read > Only, Frozen. AT_SetReadOnly and AT_SetReadWrite would simply change to the > appropriate state, and all the vacuum infrastructure would continue to > process those tables as it does today. lazy_vacuum_rel would become > responsible for tracking if there were any non-frozen tuples if it was also > attempting a freeze. If it discovered there were none, AND the table was > marked as ReadOnly, then it would change the table state to Frozen and set > relfrozenxid = InvalidTransactionId and relminxid = InvalidMultiXactId. > AT_SetReadWrite could change relfrozenxid to it's own Xid as an > optimization. Doing it that way leaves all the complicated vacuum code in > one place, and would eliminate concerns about race conditions with still > running transactions, etc. > +1 here as well. I might want to set tables to read only for reasons other than to avoid repeated freezing. (After all, the name of the command suggests it is a general purpose thing) and wouldn't want to automatically trigger a vacuum freeze in the process. Cheers, Jeff
