Re: Improving replay of XLOG_BTREE_VACUUM records

Поиск
Список
Период
Сортировка
От Vladimir Borodin
Тема Re: Improving replay of XLOG_BTREE_VACUUM records
Дата
Msg-id 5D3B0EA2-53CA-4B58-B819-5C8A57BD120E@simply.name
обсуждение исходный текст
Ответ на Re: Improving replay of XLOG_BTREE_VACUUM records  (Jim Nasby <Jim.Nasby@BlueTreble.com>)
Список pgsql-hackers
Hi, Jim.

Thanks for review.

2 мая 2015 г., в 2:10, Jim Nasby <Jim.Nasby@BlueTreble.com> написал(а):

On 5/1/15 11:19 AM, Vladimir Borodin wrote:
There are situations in which vacuuming big btree index causes stuck in
WAL replaying on hot standby servers for quite a long time. I’ve
described the problem in more details in this thread [0]. Below in that
thread Kevin Grittner proposed a good way for improving btree scans so
that btree vacuuming logic could be seriously simplified. Since I don’t
know when that may happen I’ve done a patch that makes some improvement
right now. If Kevin or someone else would expand [1] for handling all
types of btree scans, I suppose, my patch could be thrown away and
vacuuming logic should be strongly rewritten.

This looks like a good way to address this until the more significant work can be done.

I'm not a fan of "RBM_ZERO_NO_BM_VALID"; how about RBM_ZERO_BM_INVALID? or BM_NOT_VALID? Or maybe I'm just trying to impose too much English on the code; I see the logic to NO_BM_VALID…

Perhaps, RBM_ZERO_NO_BM_VALID is not so good (it makes more difficult to grep BM_VALID in code), but I don’t actually like BM_INVALID and BM_NOT_VALID, sorry :( But I also don’t insist on NO_BM_VALID, any other suggestions?


+ * RBM_ZERO_NO_BM_VALID is the same as RBM_ZERO_AND_LOCK, but does not set
+ * BM_VALID bit before returning buffer so that noone could pin it.

It would be better to explain why we want that mode. How about:

RBM_ZERO_NO_BM_VALID is the same as RBM_ZERO_AND_LOCK but does not set BM_VALID before returning the buffer. This is done to ensure that no one can pin the buffer without actually reading the buffer contents in. This is necessary while replying XLOG_BTREE_VACUUM records in hot standby.

Good point, fixed in attached patch.


+ if (mode == RBM_ZERO_NO_BM_VALID)
+ TerminateBufferIO(bufHdr, false, 0);
+ else
+ TerminateBufferIO(bufHdr, false, BM_VALID);

Simply passing in a 0 seems a bit odd to me; is there anywhere else we do that?

Yes, it is done the same way in FlushBuffer function [0]. Also comments before TerminateBufferIO say that 0 is expected value for third argument.



--
Jim Nasby, Data Architect, Blue Treble Consulting
Data in Trouble? Get it in Treble! http://BlueTreble.com


--
May the force be with you…

Вложения

В списке pgsql-hackers по дате отправления:

Предыдущее
От: Joe Wildish
Дата:
Сообщение: Re: Implementing SQL ASSERTION
Следующее
От: Sergey Grinko
Дата:
Сообщение: Re: Loss of some parts of the function definition