Обсуждение: EvictUnpinnedBuffer and buffer free list
Hi All, EvictUnpinnedBuffer() calls InvalidateVictimBuffer() followed by UnpinBuffer() before returning. None of those functions put the buffer back into the free list. Its freeNext remains set to FREENEXT_NOT_IN_LIST. I think then that buffer will never be used and lost forever. I know that that function is only meant for development or testing but even while testing something losing a buffer forever seems like a problem. The prologue of function InvalidateVictimBuffer() says "/* Helper routine for GetVictimBuffer() ". I believe that it's expected that the buffer will be allocated to some other page, and that's why it doesn't return the buffer to the free list. But in the case of EvictUnpinnedBuffer() we are not using that buffer for any page, so it must be returned to the free list. InvalidateBuffer() does that but its prologue mentions that it's supposed to be used when freeing buffers for relations and databases. I think there are two solutions 1. Use InvalidBuffer() instead of InvalidateVictimBuffer(). But I am not sure whether that's safe or what other safety measures we have to put in EvictUnpinnedBuffer() 2. Call StrategyFreeBuffer() after InvalidateVictimBuffer() Thoughts? -- Best Wishes, Ashutosh Bapat
31.01.2025 08:15, Ashutosh Bapat пишет: > Hi All, > EvictUnpinnedBuffer() calls InvalidateVictimBuffer() followed by > UnpinBuffer() before returning. None of those functions put the buffer > back into the free list. Its freeNext remains set to > FREENEXT_NOT_IN_LIST. I think then that buffer will never be used and > lost forever. I know that that function is only meant for development > or testing but even while testing something losing a buffer forever > seems like a problem. > > The prologue of function InvalidateVictimBuffer() says "/* Helper > routine for GetVictimBuffer() ". I believe that it's expected that the > buffer will be allocated to some other page, and that's why it doesn't > return the buffer to the free list. But in the case of > EvictUnpinnedBuffer() we are not using that buffer for any page, so it > must be returned to the free list. InvalidateBuffer() does that but > its prologue mentions that it's supposed to be used when freeing > buffers for relations and databases. > > I think there are two solutions > 1. Use InvalidBuffer() instead of InvalidateVictimBuffer(). But I am > not sure whether that's safe or what other safety measures we have to > put in EvictUnpinnedBuffer() > 2. Call StrategyFreeBuffer() after InvalidateVictimBuffer() > > Thoughts? Clock eviction algorithm visit every page (by StrategyGetBuffer), so it will eventually observe this buffer and use it for new page.
On Fri, Jan 31, 2025 at 2:19 PM Yura Sokolov <y.sokolov@postgrespro.ru> wrote: > > 31.01.2025 08:15, Ashutosh Bapat пишет: > > Hi All, > > EvictUnpinnedBuffer() calls InvalidateVictimBuffer() followed by > > UnpinBuffer() before returning. None of those functions put the buffer > > back into the free list. Its freeNext remains set to > > FREENEXT_NOT_IN_LIST. I think then that buffer will never be used and > > lost forever. I know that that function is only meant for development > > or testing but even while testing something losing a buffer forever > > seems like a problem. > > > > The prologue of function InvalidateVictimBuffer() says "/* Helper > > routine for GetVictimBuffer() ". I believe that it's expected that the > > buffer will be allocated to some other page, and that's why it doesn't > > return the buffer to the free list. But in the case of > > EvictUnpinnedBuffer() we are not using that buffer for any page, so it > > must be returned to the free list. InvalidateBuffer() does that but > > its prologue mentions that it's supposed to be used when freeing > > buffers for relations and databases. > > > > I think there are two solutions > > 1. Use InvalidBuffer() instead of InvalidateVictimBuffer(). But I am > > not sure whether that's safe or what other safety measures we have to > > put in EvictUnpinnedBuffer() > > 2. Call StrategyFreeBuffer() after InvalidateVictimBuffer() > > > > Thoughts? > > Clock eviction algorithm visit every page (by StrategyGetBuffer), so it > will eventually observe this buffer and use it for new page. > yes, that's correct. That does reduce the intensity of problem very much. However, a backend which would otherwise be able to get a buffer from freelist will be forced to evict some other useful buffer. This may show unexpected results even in testing/development experiments being carried out. -- Best Wishes, Ashutosh Bapat
On Fri, Jan 31, 2025 at 12:16 AM Ashutosh Bapat <ashutosh.bapat.oss@gmail.com> wrote: > > Hi All, > EvictUnpinnedBuffer() calls InvalidateVictimBuffer() followed by > UnpinBuffer() before returning. None of those functions put the buffer > back into the free list. Its freeNext remains set to > FREENEXT_NOT_IN_LIST. I think then that buffer will never be used and > lost forever. I know that that function is only meant for development > or testing but even while testing something losing a buffer forever > seems like a problem. As Yura says in a response to this mail, it won't be lost forever. Those buffers can be found in the clocksweep. Very few operations put buffers on the freelist. In the past, there has been concern over contention on the StrategyControl->buffer_strategy_lock. I found a few discussions about this -- mostly in relation to having bgwriter put buffers on the freelist. [1] and some other mentions about putting buffers on the freelist to improve buffer eviction performance [2] and [3]. These are all ancient though. I don't have an explicit issue with EvictUnpinnedBuffer() putting buffers on the freelist -- it seems like that could be fine. But since it is for testing/development, I don't see what benefits it will have to users. It sounds like you saw issues when developing -- what kinds of issues? > The prologue of function InvalidateVictimBuffer() says "/* Helper > routine for GetVictimBuffer() ". I believe that it's expected that the > buffer will be allocated to some other page, and that's why it doesn't > return the buffer to the free list. But in the case of > EvictUnpinnedBuffer() we are not using that buffer for any page, so it > must be returned to the free list. InvalidateBuffer() does that but > its prologue mentions that it's supposed to be used when freeing > buffers for relations and databases. > > I think there are two solutions > 1. Use InvalidBuffer() instead of InvalidateVictimBuffer(). But I am > not sure whether that's safe or what other safety measures we have to > put in EvictUnpinnedBuffer() I don't really think we can do this. InvalidateBuffer() waits forever to be able to put the buffer on the freelist. That's because it is only used when dropping a relation or database. So it can assume (as it says in the comments above WaitIO()) that the only reason the buffer will be pinned is if someone else is flushing out the page. It will always retry -- since the relation is being dropped, no one else could be trying to concurrently access it to read it. You can't make this assumption in EvictUnpinnedBuffer(). > 2. Call StrategyFreeBuffer() after InvalidateVictimBuffer() I don't know exactly what would be required to make this work, but it seems reasonable to try. The only places StrategyFreeBuffer() is used is 1) InvalidateBuffer() and 2) when doing relation extension. In the first case, we know no one can know about the buffer because we waited until all pins were released and the buffer is part of a relation that is being dropped. In the second case, I think the buffers we add to the freelist are also ones that no one can know about yet because the extension hasn't completed. I'm fuzzy on the details here, so I would defer to Andres. Anyway, my gut feeling is that we have to do something to make calling StrategyFreeBuffer() safe to do in EvictUnpinnedBuffer(), but I don't know what it is. - Melanie [1] https://www.postgresql.org/message-id/flat/00cc01ce5240%242da362b0%2488ea2810%24%40kapila%40huawei.com#7f6178995e3597bca130e8b89ca6f7ab [2] https://www.postgresql.org/message-id/flat/CA%2BTgmoaP1PtRSkU0%3Dioi4hRxqCBzrNP9JV1L0YdkBp42PESSzw%40mail.gmail.com#9a4518544965d3e24f86ab79c4270810 [3] https://www.postgresql.org/message-id/flat/20140911110143.GV24649%40awork2.anarazel.de#9fd99d03652934f0d897ee88e68754ba
Thanks a lot Melanie for a very detailed response, a good reference to pin. On Fri, Jan 31, 2025 at 8:20 PM Melanie Plageman <melanieplageman@gmail.com> wrote: > > I don't have an explicit issue with EvictUnpinnedBuffer() putting > buffers on the freelist -- it seems like that could be fine. But since > it is for testing/development, I don't see what benefits it will have > to users. It sounds like you saw issues when developing -- what kinds > of issues? I can't say it was issue, may be an expectation mismatch. In my experiment where the entire buffer pool was full, I was expecting the evicted buffer to be available immediately for the next page request. I didn't expect another eviction. I kinda thought that the buffer was lost, but it was returned. The next thing I tried was to evict many buffers together using EvictUnpinnedBuffer() and those buffers took a long time to return to the pool because clock sweep wasn't as fast as the eviction. But that's not a regular scenario, so may be current behaviour is okay to avoid the lock contention. > > > The prologue of function InvalidateVictimBuffer() says "/* Helper > > routine for GetVictimBuffer() ". I believe that it's expected that the > > buffer will be allocated to some other page, and that's why it doesn't > > return the buffer to the free list. But in the case of > > EvictUnpinnedBuffer() we are not using that buffer for any page, so it > > must be returned to the free list. InvalidateBuffer() does that but > > its prologue mentions that it's supposed to be used when freeing > > buffers for relations and databases. > > > > I think there are two solutions > > 1. Use InvalidBuffer() instead of InvalidateVictimBuffer(). But I am > > not sure whether that's safe or what other safety measures we have to > > put in EvictUnpinnedBuffer() > > I don't really think we can do this. InvalidateBuffer() waits forever > to be able to put the buffer on the freelist. That's because it is > only used when dropping a relation or database. So it can assume (as > it says in the comments above WaitIO()) that the only reason the > buffer will be pinned is if someone else is flushing out the page. It > will always retry -- since the relation is being dropped, no one else > could be trying to concurrently access it to read it. You can't make > this assumption in EvictUnpinnedBuffer(). Thanks for the explanation. This option is ruled out then. > > > 2. Call StrategyFreeBuffer() after InvalidateVictimBuffer() > > I don't know exactly what would be required to make this work, but it > seems reasonable to try. The only places StrategyFreeBuffer() is used > is 1) InvalidateBuffer() and 2) when doing relation extension. In the > first case, we know no one can know about the buffer because we waited > until all pins were released and the buffer is part of a relation that > is being dropped. In the second case, I think the buffers we add to > the freelist are also ones that no one can know about yet because the > extension hasn't completed. I'm fuzzy on the details here, so I would > defer to Andres. > > Anyway, my gut feeling is that we have to do something to make calling > StrategyFreeBuffer() safe to do in EvictUnpinnedBuffer(), but I don't > know what it is. I think we may enhance the pg_buffercache_evict() function to put it back in the freelist; the behaviour being controlled by an argument flag. I haven't explored the feasibility yet. That will leave EvictUnpinnedBuffer() as is. -- Best Wishes, Ashutosh Bapat