Pavan Deolasee wrote:
> Please see the attached version 11 of HOT patch
Thanks!
One wrinkle in the patch is how the ResultRelInfo-struct is passed to
heap_update, and on to heap_check_idxupdate, to check any indexed
columns have changed. I think that's a modularity violation, heap_update
really shouldn't have to deal with ResultRelInfo, that belongs in the
executor. When we add support for expression and partial indexes,
heap_check_idxupdate will need even more executor machinery to be able
to evaluate expressions.
In heap_page_prune_defrag, it would be better to do the test for
BufferIsLockedForCleanup right after acquiring the lock. The longer the
delay between those steps, the bigger the chances that someone pins the
page and starts to wait for the buffer lock, making us think that we
didn't get the cleanup lock, though we actually did. Maybe a nicer
solution would be to have another version of ConditionalLockBuffer with
three different return values: didn't get lock, got exclusive lock, or
got cleanup lock.
It's not necessary to WAL-log the unused-array that
PageRepairFragmentation returns. In replay, a call to
PageRepairFragmentation will come to the same conclusion about which
line pointers are not used. It would also be better if we didn't emit a
separate WAL record for defraging a page, if we also prune it at the
same time. I'm not that worried about WAL usage in general, but that
seems simple enough to fix.
--
Heikki Linnakangas
EnterpriseDB http://www.enterprisedb.com