Heikki Linnakangas wrote:
Another thing that does need to be fixed, is the way that the extension
and truncation of the visibility map is handled; that's broken in the
current patch. I started working on the patch a long time ago, before
the FSM rewrite was finished, and haven't gotten around fixing that part
yet. We already solved it for the FSM, so we could just follow that
pattern. The way we solved truncation in the FSM was to write a separate
WAL record with the new heap size, but perhaps we want to revisit that
decision, instead of adding again new code to write a third WAL record,
for truncation of the visibility map. smgrtruncate() writes a WAL record
of its own, if any full blocks are truncated away of the FSM, but we
needed a WAL record even if no full blocks are truncated from the FSM
file, because the "tail" of the last remaining FSM page, representing
the truncated away heap pages, still needs to cleared. Visibility map
has the same problem.
One proposal was to piggyback on the smgrtruncate() WAL-record, and call
FreeSpaceMapTruncateRel from smgr_redo(). I considered that ugly from a
modularity point of view; smgr.c shouldn't be calling higher-level
functions. But maybe it wouldn't be that bad, after all. Or, we could
remove WAL-logging from smgrtruncate() altogether, and move it to
RelationTruncate() or another higher-level function, and handle the
WAL-logging and replay there.
In preparation for the visibility map patch, I revisited the truncation
issue, and hacked together a patch to piggyback the FSM truncation to
the main fork smgr truncation WAL record. I moved the WAL-logging from
smgrtruncate() to RelationTruncate(). There's a new flag to
RelationTruncate indicating whether the FSM should be truncated too, and
only one truncation WAL record is written for the operation.
That does seem cleaner than the current approach where the FSM writes a
separate WAL record just to clear the bits of the last remaining FSM
page. I had to move RelationTruncate() to smgr.c, because I don't think
a function in bufmgr.c should be doing WAL-logging. However,
RelationTruncate really doesn't belong in smgr.c either. Also, now that
smgrtruncate doesn't write its own WAL record, it doesn't seem right for
smgrcreate to be doing that either.
So, I think I'll take this one step forward, and move RelationTruncate()
to a new higher level file, e.g. src/backend/catalog/storage.c, and also
create a new RelationCreateStorage() function that calls smgrcreate(),
and move the WAL-logging from smgrcreate() to RelationCreateStorage().
So, we'll have two functions in a new file:
/* Create physical storage for a relation. If 'fsm' is true, an FSM fork
is also created */
RelationCreateStorage(Relation rel, bool fsm)
/* Truncate the relation to 'nblocks' blocks. If 'fsm' is true, the FSM
is also truncated */
RelationTruncate(Relation rel, BlockNumber nblocks, bool fsm)
The next question is whether the "pending rel deletion" stuff in smgr.c
should be moved to the new file too. It seems like it would belong there
better. That would leave smgr.c as a very thin wrapper around md.c
--
Heikki Linnakangas
EnterpriseDB http://www.enterprisedb.com
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
