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? I don't think it's safe to leave tuples as dead either, even if they're hinted. The hint may not be written. Also, the patch seems to be completely ignoring actually freezing the toast relation; I can't see how that's actually safe.

I'd feel a heck of a lot safer if any time heap_prepare_freeze_tuple returned false we did a second check on the tuple to ensure it was truly frozen.

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.

BTW, you also need to put things in place to ensure it's impossible to unfreeze a tuple in a relation that's marked ReadOnly or Frozen. I'm not sure what the right way to do that would be.
--
Jim Nasby, Data Architect, Blue Treble Consulting
Data in Trouble? Get it in Treble! http://BlueTreble.com


--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Reply via email to