Thread: Avoiding unnecessary writes during relation drop and truncate

Currently, in places like heap_drop_with_catalog, we issue a
FlushRelationBuffers() call followed by smgrscheduleunlink().
The latter doesn't actually do anything right away, but schedules
a file unlink to occur after transaction commit.

It strikes me that the FlushRelationBuffers call is unnecessary and
causes useless I/O, namely writing out pages into a file that's
about to be deleted anyway.  If we simply removed it then any buffers
belonging to the victim relation would stay in memory until commit;
then they'd be dropped *without* write by the smgr unlink operation
(which already calls DropRelFileNodeBuffers).

This doesn't cause any problems with rolling back the transaction before
commit; we can perfectly well leave dirty pages in the buffer pool in
that case.  About the only downside I can see is that the Flush allows
buffer pages to be freed slightly sooner, and hence possibly used for
something else later in the same transaction ... but that's hardly worth
the cost of writing data that might not need to be written at all.

Similar remarks apply to the partial FlushRelationBuffers calls that are
currently done just before partial or full truncation of a relation ---
except that those are even sillier, because we are writing data that we
are definitely going to tell the kernel to forget about immediately
afterward.  We should just drop any buffers that are past the truncation
point.  smgrtruncate isn't roll-back-able anyway, so the caller already
has to be certain that the pages aren't going to be needed anymore
regardless of any subsequent rollback.

Can anyone see a flaw in this logic?

I think that the FlushRelationBuffers calls associated with deletion
are leftover from a time when we actually deleted the target file
immediately (ie, back when DROP TABLE wasn't rollback-safe).  The
ones associated with truncation were probably just modeled on the
deletion logic without sufficient thought.
        regards, tom lane

On Sat, 2005-03-19 at 18:53 -0500, Tom Lane wrote:
> Currently, in places like heap_drop_with_catalog, we issue a
> FlushRelationBuffers() call followed by smgrscheduleunlink().
> The latter doesn't actually do anything right away, but schedules
> a file unlink to occur after transaction commit.
> 
> It strikes me that the FlushRelationBuffers call is unnecessary and
> causes useless I/O, namely writing out pages into a file that's
> about to be deleted anyway.  If we simply removed it then any buffers
> belonging to the victim relation would stay in memory until commit;
> then they'd be dropped *without* write by the smgr unlink operation
> (which already calls DropRelFileNodeBuffers).
> 
> This doesn't cause any problems with rolling back the transaction before
> commit; we can perfectly well leave dirty pages in the buffer pool in
> that case.  About the only downside I can see is that the Flush allows
> buffer pages to be freed slightly sooner, and hence possibly used for
> something else later in the same transaction ... but that's hardly worth
> the cost of writing data that might not need to be written at all.
> 
> Similar remarks apply to the partial FlushRelationBuffers calls that are
> currently done just before partial or full truncation of a relation ---
> except that those are even sillier, because we are writing data that we
> are definitely going to tell the kernel to forget about immediately
> afterward.  We should just drop any buffers that are past the truncation
> point.  smgrtruncate isn't roll-back-able anyway, so the caller already
> has to be certain that the pages aren't going to be needed anymore
> regardless of any subsequent rollback.
> 
> Can anyone see a flaw in this logic?
> 
> I think that the FlushRelationBuffers calls associated with deletion
> are leftover from a time when we actually deleted the target file
> immediately (ie, back when DROP TABLE wasn't rollback-safe).  The
> ones associated with truncation were probably just modeled on the
> deletion logic without sufficient thought.

Yes, I think FlushRelationBuffers can be simply removed. I'd wanted to
get rid of it before, but hadn't seen how to.

Not sure I understand all of your other comments though. If all you mean
to do is to simply remove the call, then please ignore this:

ISTM that buffers belonging to the victim relation would not necessarily
stay in memory. If they were pinned still, then there would be a lock
that would have prevented the DROP TABLE from going through. The buffers
are not pinned and so will stay in memory until aged out by the dirty
write process. You're right, there's no particular benefit of doing them
earlier - we have a bgwriter now that can do that for us when it comes
to it.

Removing FlushRelationBuffers in those circumstances will save a scan of
shared_buffers, but will it save I/O? Perhaps not, but I care more about
the O(N) operation on shared_buffers than I do about the I/O.

Best Regards, Simon Riggs

Simon Riggs <simon@2ndquadrant.com> writes:
> ISTM that buffers belonging to the victim relation would not necessarily
> stay in memory.

Right.  They'd be unpinned and therefore candidates for being written
out and recycled.  So we *might* write them before they are dropped.
That's still better than *definitely* writing them.

The particular case that annoyed me into thinking about this was
using strace to watch a backend manipulate a temp table, and seeing
it carefully write out a bunch of pages from local buffers just
before it dropped the temp table :-(.  For local buffers there are
no checkpoints nor bgwriter and so accumulation of a lot of dirty
pages can be expected.  It didn't matter all that much before
yesterday, with the size of the local buffer pool hardwired at 64,
but when there are hundreds or thousands of local buffers it'll
be important to suppress useless writes.

> Removing FlushRelationBuffers in those circumstances will save a scan of
> shared_buffers, but will it save I/O? Perhaps not, but I care more about
> the O(N) operation on shared_buffers than I do about the I/O.

Realistically, wasted I/O costs more.  But yeah, saving one scan of the
buffer arena is a nice side benefit.
        regards, tom lane

"Tom Lane" <tgl@sss.pgh.pa.us> writes
> It strikes me that the FlushRelationBuffers call is unnecessary and
> causes useless I/O, namely writing out pages into a file that's
> about to be deleted anyway.  If we simply removed it then any buffers
> belonging to the victim relation would stay in memory until commit;
> then they'd be dropped *without* write by the smgr unlink operation
> (which already calls DropRelFileNodeBuffers).
>

In my understanding, DropRelFileNodeBuffers is done in post-commit stage.
The call sequences are:
...AtEOXact_Inval(true);
smgrDoPendingDeletes(true);...

What if AtEOXact_Inval() fails (though the chance is slim)? Does that mean
that smgrDoPendingDeletes() -> DropRelFileNodeBuffers can never get
executed, which means we can never "dropped without write" the buffers
belonging to the victim relation? So when the BgWrite sweeps, it will write
those buffers to a non-logically-existed file?

Regards,
Qingqing

"Qingqing Zhou" <zhouqq@cs.toronto.edu> writes:
> What if AtEOXact_Inval() fails (though the chance is slim)? Does that mean
> that smgrDoPendingDeletes() -> DropRelFileNodeBuffers can never get
> executed, which means we can never "dropped without write" the buffers
> belonging to the victim relation? So when the BgWrite sweeps, it will write
> those buffers to a non-logically-existed file?

So it'll get an error ... this scenario doesn't strike me as any worse
than any other problem occuring in post-commit cleanup.  The locks left
around by the not-cleaned-up transaction would probably be a bigger
issue, for example.
        regards, tom lane

"Tom Lane" <tgl@sss.pgh.pa.us> writes
> So it'll get an error ... this scenario doesn't strike me as any worse
> than any other problem occuring in post-commit cleanup.  The locks left
> around by the not-cleaned-up transaction would probably be a bigger
> issue, for example.

Yes, the result is acceptable. BTW, what about other post-commit cleanup
errors? Do we have any idea to avoid/alleviate them?

Regards,
Qingqing

On Sun, 2005-03-20 at 12:28 -0500, Tom Lane wrote:
> > Removing FlushRelationBuffers in those circumstances will save a scan of
> > shared_buffers, but will it save I/O? Perhaps not, but I care more about
> > the O(N) operation on shared_buffers than I do about the I/O.
> 
> Realistically, wasted I/O costs more.  But yeah, saving one scan of the
> buffer arena is a nice side benefit.

3-4 years ago, I'd have said definitely true.

I'm seeing servers with enough RAM to swallow databases whole and people
willing to allocate that all to their DBMS. People try to avoid the I/O,
but do that by upping the allocations.

I guess my vision is for these kind of parameters and much beyond...
shared_buffers = 4000000
max_fsm_pages = 1000000

so the O(N) stuff matters... but I realise I'm following Amdahl's Law to
the point where there's nothing significant left to care about.

Best Regards, Simon Riggs