Re: [PATCH] Full support for index LP_DEAD hint bits on standby

2022-04-10 Thread Peter Geoghegan
On Thu, Mar 31, 2022 at 4:57 PM Michail Nikolaev wrote: > I remember you had an idea about using the LP_REDIRECT bit in btree > indexes as some kind of “recently dead” flag (1). > Is this idea still in progress? Maybe an additional bit could provide > a space for a better solution. I think that

Re: [PATCH] Full support for index LP_DEAD hint bits on standby

2022-03-31 Thread Michail Nikolaev
Hello, David. Thanks for your review! > As a specific recommendation here - submit patches with a complete commit > message. > Tweak it for each new version so that any prior discussion that informed the > general design of > the patch is reflected in the commit message. Yes, agreed. Applied

Re: [PATCH] Full support for index LP_DEAD hint bits on standby

2022-03-31 Thread Michail Nikolaev
Hello, Peter. > The simple answer is: I don't know. I could probably come up with a > better answer than that, but it would take real effort, and time. I remember you had an idea about using the LP_REDIRECT bit in btree indexes as some kind of “recently dead” flag (1). Is this idea still in

Re: [PATCH] Full support for index LP_DEAD hint bits on standby

2022-03-29 Thread David G. Johnston
On Tue, Mar 22, 2022 at 6:52 AM Michail Nikolaev wrote: > Hello, Andres. > > > Fails to apply at the moment: http://cfbot.cputube.org/patch_37_2947.log > > Thanks for notifying me. BTW, some kind of automatic email in case of > status change could be very helpful. > > > Marked as waiting for

Re: [PATCH] Full support for index LP_DEAD hint bits on standby

2022-03-29 Thread David G. Johnston
On Tue, Mar 29, 2022 at 5:20 PM Peter Geoghegan wrote: > On Tue, Mar 29, 2022 at 4:55 AM Michail Nikolaev > wrote: > > I think that you could do a better job of explaining and promoting the > problem that you're trying to solve here. Emphasis on the problem, not > so much the solution. As a

Re: [PATCH] Full support for index LP_DEAD hint bits on standby

2022-03-29 Thread Peter Geoghegan
On Tue, Mar 29, 2022 at 4:55 AM Michail Nikolaev wrote: > > Overall, I think that this patch has serious design flaws, and that > > this issue is really just a symptom of a bigger problem. > > Could you please advise me on something? The ways I see: > * give up :) I would never tell anybody to

Re: [PATCH] Full support for index LP_DEAD hint bits on standby

2022-03-29 Thread Michail Nikolaev
UPD: > I was thinking it is safe to have additional hint bits > on primary, but it seems like no. Oh, sorry for the mistake, it is about standby of course. > BTW I am wondering if it is possible > to achieve the same situation by pg_rewind and standby promotion… Looks like it is impossible,

Re: [PATCH] Full support for index LP_DEAD hint bits on standby

2022-03-29 Thread Michail Nikolaev
Hello, Peter. Thanks for your review! > I doubt that the patch's use of pg_memory_barrier() in places like > _bt_killitems() is correct. There is no way to know for sure if this > novel new lockless algorithm is correct or not, since it isn't > explained anywhere. The memory barrier is used

Re: [PATCH] Full support for index LP_DEAD hint bits on standby

2022-03-29 Thread Michail Nikolaev
Hello, Greg. > I'm seeing a recovery test failure. Not sure if this represents an > actual bug or just a test that needs to be adjusted for the new > behaviour. Thanks for notifying me. It is a failure of a test added in the patch. It is a little hard to make it stable (because it depends on

Re: [PATCH] Full support for index LP_DEAD hint bits on standby

2022-03-28 Thread Peter Geoghegan
On Mon, Mar 28, 2022 at 1:23 PM Peter Geoghegan wrote: > I doubt that the patch's use of pg_memory_barrier() in places like > _bt_killitems() is correct. I also doubt that posting list splits are handled correctly. If there is an LP_DEAD bit set on a posting list on the primary, and we need to

Re: [PATCH] Full support for index LP_DEAD hint bits on standby

2022-03-28 Thread Peter Geoghegan
On Mon, Mar 28, 2022 at 12:40 PM Greg Stark wrote: > I'm seeing a recovery test failure. Not sure if this represents an > actual bug or just a test that needs to be adjusted for the new > behaviour. > > https://cirrus-ci.com/task/5711008294502400 I doubt that the patch's use of

Re: [PATCH] Full support for index LP_DEAD hint bits on standby

2022-03-28 Thread Greg Stark
On Tue, 22 Mar 2022 at 09:52, Michail Nikolaev wrote: > > Thanks for notifying me. BTW, some kind of automatic email in case of > status change could be very helpful. I agree but realize the cfbot is quite new and I guess the priority is to work out any kinks before spamming people with false

Re: [PATCH] Full support for index LP_DEAD hint bits on standby

2022-03-22 Thread Michail Nikolaev
Hello, Andres. > Fails to apply at the moment: http://cfbot.cputube.org/patch_37_2947.log Thanks for notifying me. BTW, some kind of automatic email in case of status change could be very helpful. > Marked as waiting for author. New version is attached, build is passing

Re: [PATCH] Full support for index LP_DEAD hint bits on standby

2022-03-21 Thread Andres Freund
Hi, On 2022-01-25 19:21:01 +0800, Julien Rouhaud wrote: > I rebased the pathset in attached v9. Please double check that I didn't miss > anything in the rebase. Fails to apply at the moment: http://cfbot.cputube.org/patch_37_2947.log Marked as waiting for author. - Andres

Re: [PATCH] Full support for index LP_DEAD hint bits on standby

2022-01-25 Thread Michail Nikolaev
Hello, Julien. > I rebased the pathset in attached v9. Please double check that I didn't miss > anything in the rebase. Thanks a lot for your help. > I will let you mark the patch as Ready for Committer once you validate that > the > rebase was ok. Yes, rebase looks good. Best regards,

Re: [PATCH] Full support for index LP_DEAD hint bits on standby

2022-01-25 Thread Julien Rouhaud
Hi, On Tue, Jan 25, 2022 at 07:21:01PM +0800, Julien Rouhaud wrote: > > > > I'll move entry back to "Ready for Committer" once it passes tests. > > It looks like you didn't fetch the latest upstream commits in a while as this > version is still conflicting with 7a5f6b474 (Make logical decoding

Re: [PATCH] Full support for index LP_DEAD hint bits on standby

2022-01-25 Thread Julien Rouhaud
Hi, On Mon, Jan 24, 2022 at 10:33:43AM +0300, Michail Nikolaev wrote: > > Thanks for your attention. > After some investigation, I think I have found the problem. It is > caused by XLOG_RUNNING_XACTS at an undetermined moment (some test > parts rely on it). > > Now test waits for

Re: [PATCH] Full support for index LP_DEAD hint bits on standby

2022-01-23 Thread Michail Nikolaev
Hello, Justin. Thanks for your attention. After some investigation, I think I have found the problem. It is caused by XLOG_RUNNING_XACTS at an undetermined moment (some test parts rely on it). Now test waits for XLOG_RUNNING_XACTS to happen (maximum is 15s) and proceed forward. I'll move entry

Re: [PATCH] Full support for index LP_DEAD hint bits on standby

2022-01-15 Thread Justin Pryzby
On Sat, Jan 15, 2022 at 08:39:14PM +0300, Michail Nikolaev wrote: > Hello, Junien. > > Thanks for your attention. > > > The cfbot reports that this patch is currently failing at least on > > Linux and Windows, e.g. https://cirrus-ci.com/task/6532060239101952. > > Fixed. It was the issue with

Re: [PATCH] Full support for index LP_DEAD hint bits on standby

2022-01-15 Thread Michail Nikolaev
Hello, Junien. Thanks for your attention. > The cfbot reports that this patch is currently failing at least on > Linux and Windows, e.g. https://cirrus-ci.com/task/6532060239101952. Fixed. It was the issue with the test - hangs on Windows because of psql + spurious vacuum sometimes. > I'm

Re: [PATCH] Full support for index LP_DEAD hint bits on standby

2022-01-11 Thread Julien Rouhaud
Hi, On Wed, Nov 10, 2021 at 3:06 AM Michail Nikolaev wrote: > > > Attached is a proposal for a minor addition that would make sense to me, add > > it if you think it's appropriate. > > Added. Also, I updated the documentation a little. > > > I have changed approach, so it is better to start from

Re: [PATCH] Full support for index LP_DEAD hint bits on standby

2021-11-09 Thread Michail Nikolaev
Hello. > Attached is a proposal for a minor addition that would make sense to me, add > it if you think it's appropriate. Added. Also, I updated the documentation a little. > I have changed approach, so it is better to start from this email: Oops, I was thinking the comments feature in the

Re: [PATCH] Full support for index LP_DEAD hint bits on standby

2021-11-09 Thread Michail Nikolaev
Woo-hoo :) > Attached is a proposal for a minor addition that would make sense to me, add > it if you think it's appropriate. Yes, I'll add to the patch. > I think I've said enough, changing the status to "ready for committer" :-) Thanks a lot for your help and attention! Best regards,

Re: [PATCH] Full support for index LP_DEAD hint bits on standby

2021-11-09 Thread Michail Nikolaev
I have changed approach, so it is better to start from this email: https://www.postgresql.org/message-id/flat/CANtu0ohHu1r1xQfTzEJuxeaOMYncG7xRxUQWdH%3DcMXZSf%2Bnzvg%40mail.gmail.com#4c81a4d623d8152f5e8889e97e750eec

Re: [PATCH] Full support for index LP_DEAD hint bits on standby

2021-11-09 Thread Antonin Houska
Michail Nikolaev wrote: > > I understand that the RR snapshot is used to check the MVCC behaviour, > > however > > this comment seems to indicate that the RR snapshot should also prevent the > > standb from setting the hint bits. > > # Make sure previous queries not set the hints on standby

Re: [PATCH] Full support for index LP_DEAD hint bits on standby

2021-11-05 Thread Michail Nikolaev
Hello, Antonin. Thanks for pushing it forward. > I understand that the RR snapshot is used to check the MVCC behaviour, however > this comment seems to indicate that the RR snapshot should also prevent the > standb from setting the hint bits. > # Make sure previous queries not set the hints on

Re: [PATCH] Full support for index LP_DEAD hint bits on standby

2021-11-03 Thread Antonin Houska
Michail Nikolaev wrote: > > * Is the purpose of the repeatable read (RR) snapshot to test that > > heap_hot_search_buffer() does not set deadness->all_dead if some > > transaction > > can still see a tuple of the chain? > > The main purpose is to test xactStartedInRecovery logic after the

Re: [PATCH] Full support for index LP_DEAD hint bits on standby

2021-09-29 Thread Michail Nikolaev
Hello, Antonin. > I'm trying to continue the review, sorry for the delay. Following are a few > question about the code: Thanks for the review :) And sorry for the delay too :) > * Does the masking need to happen in the AM code, e.g. _bt_killitems()? > I'd expect that the RmgrData.rm_fpi_mask

Re: [PATCH] Full support for index LP_DEAD hint bits on standby

2021-09-20 Thread Antonin Houska
Michail Nikolaev wrote: > Hello. > > Added a check for standby promotion with the long transaction to the > test (code and docs are unchanged). I'm trying to continue the review, sorry for the delay. Following are a few question about the code: * Does the masking need to happen in the AM

Re: [PATCH] Full support for index LP_DEAD hint bits on standby

2021-05-13 Thread Michail Nikolaev
Hello. Added a check for standby promotion with the long transaction to the test (code and docs are unchanged). Thanks, Michail. From c5e1053805c537b50b0922151bcf127754500adb Mon Sep 17 00:00:00 2001 From: Michail Nikolaev Date: Fri, 14 May 2021 00:32:30 +0300 Subject: [PATCH v3 3/4] test ---

Re: [PATCH] Full support for index LP_DEAD hint bits on standby

2021-05-12 Thread Michail Nikolaev
Hello, Antonin. > My review that started in [1] continues here. Thanks a lot for the review. > (Please note that code.patch does not apply to the current master branch.) Rebased. > Especially for the problem discussed in [1] it should be > explained what would happen if kill_prior_tuple_min_lsn

Re: [PATCH] Full support for index LP_DEAD hint bits on standby

2021-05-10 Thread Antonin Houska
Michail Nikolaev wrote: > After some correspondence with Peter Geoghegan (1) and his ideas, I > have reworked the patch a lot and now it is much more simple with even > better performance (no new WAL or conflict resolution, hot standby > feedback is unrelated). My review that started in [1]

Re: [PATCH] Full support for index LP_DEAD hint bits on standby

2021-05-10 Thread Antonin Houska
Michail Nikolaev wrote: > > Sorry, I missed the fact that your example can be executed inside BEGIN - > > END > > block, in which case minRecoveryPoint won't advance after each command. > > No, the block is not executed as a single transaction, all commands > are separated transactions (see

Re: [PATCH] Full support for index LP_DEAD hint bits on standby

2021-05-10 Thread Michail Nikolaev
Hello, Antonin. > Sorry, I missed the fact that your example can be executed inside BEGIN - END > block, in which case minRecoveryPoint won't advance after each command. No, the block is not executed as a single transaction, all commands are separated transactions (see below) > Actually I think

Re: [PATCH] Full support for index LP_DEAD hint bits on standby

2021-05-10 Thread Antonin Houska
Michail Nikolaev wrote: > Hello, Antonin. > > > I'm trying to review the patch, but not sure if I understand this problem, > > please see my comment below. > > Thanks a lot for your attention. It is strongly recommended to look at > version N3 (1) because it is a much more elegant, easy, and

Re: [PATCH] Full support for index LP_DEAD hint bits on standby

2021-05-07 Thread Michail Nikolaev
Hello, Antonin. > I'm trying to review the patch, but not sure if I understand this problem, > please see my comment below. Thanks a lot for your attention. It is strongly recommended to look at version N3 (1) because it is a much more elegant, easy, and reliable solution :) But the

Re: [PATCH] Full support for index LP_DEAD hint bits on standby

2021-05-06 Thread Antonin Houska
I'm trying to review the patch, but not sure if I understand this problem, please see my comment below. Michail Nikolaev wrote: > Oh, I just realized that it seems like I was too naive to allow > standby to set LP_DEAD bits this way. > There is a possible consistency problem in the case of low

Re: [PATCH] Full support for index LP_DEAD hint bits on standby

2021-01-27 Thread Michail Nikolaev
Hello, hackers. I think I was able to fix the issue related to minRecoveryPoint and crash recovery. To make sure standby will be consistent after crash recovery, we need to take the current value of minRecoveryPoint into account while setting LP_DEAD hints (almost the same way as it is done for

Re: [PATCH] Full support for index LP_DEAD hint bits on standby

2021-01-21 Thread Michail Nikolaev
Hello, everyone. Oh, I just realized that it seems like I was too naive to allow standby to set LP_DEAD bits this way. There is a possible consistency problem in the case of low minRecoveryPoint value (because hint bits do not move PageLSN forward). Something like this: LSN=10 STANDBY INSERTS

[PATCH] Full support for index LP_DEAD hint bits on standby

2021-01-18 Thread Michail Nikolaev
Hello, hackers. [ABSTRACT] Execution of queries to hot standby is one of the most popular ways to scale application workload. Most of the modern Postgres installations have two standby nodes for high-availability support. So, utilization of replica's CPU seems to be a reasonable idea. At the