Re: [HACKERS] Some other things about contrib/bloom and generic_xlog.c

2016-04-12 Thread Robert Haas
On Tue, Apr 12, 2016 at 9:36 AM, Craig Ringer wrote: > Repeating the mapping at each checkpoint sounds pretty reasonable and means > we always know what we need. There's no need to bloat each record with an > extension name and no need for any kind of ugly global

Re: [HACKERS] Some other things about contrib/bloom and generic_xlog.c

2016-04-12 Thread Alexander Korotkov
On Tue, Apr 12, 2016 at 6:42 PM, Tom Lane wrote: > Alexander Korotkov writes: > > I agree with both of these ideas. Patch is attached. > > Pushed with minor cleanup. > Great, thanks! -- Alexander Korotkov Postgres Professional:

Re: [HACKERS] Some other things about contrib/bloom and generic_xlog.c

2016-04-12 Thread Tom Lane
Alexander Korotkov writes: > I agree with both of these ideas. Patch is attached. Pushed with minor cleanup. regards, tom lane -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription:

Re: [HACKERS] Some other things about contrib/bloom and generic_xlog.c

2016-04-12 Thread Alexander Korotkov
On Tue, Apr 12, 2016 at 6:34 PM, Teodor Sigaev wrote: > I agree with both of these ideas. Patch is attached. >> > > Hmm, you accidentally forget to change flag type of GenericXLogRegister in > contrib/bloom signature. Fixed. -- Alexander Korotkov Postgres Professional:

Re: [HACKERS] Some other things about contrib/bloom and generic_xlog.c

2016-04-12 Thread Teodor Sigaev
I agree with both of these ideas. Patch is attached. Hmm, you accidentally forget to change flag type of GenericXLogRegister in contrib/bloom signature. -- Teodor Sigaev E-mail: teo...@sigaev.ru WWW:

Re: [HACKERS] Some other things about contrib/bloom and generic_xlog.c

2016-04-12 Thread Tom Lane
Alexander Korotkov writes: > Attached patch is intended to fix this. It zeroes "hole" in both > GenericXLogFinish() and generic_redo(). Pushed. I rewrote the comments a bit and modified GenericXLogFinish so that it doesn't copy data only to overwrite it with zeroes.

Re: [HACKERS] Some other things about contrib/bloom and generic_xlog.c

2016-04-12 Thread Craig Ringer
On 12 April 2016 at 20:48, Robert Haas wrote: > > So how do we look at a generic log record, say "ok, the upstream that > wrote > > this says it needs to invoke registered generic xlog hook 42, which is > > from at " ? > > Record enough information in WAL that the rmgrs

Re: [HACKERS] Some other things about contrib/bloom and generic_xlog.c

2016-04-12 Thread Alexander Korotkov
On Tue, Apr 12, 2016 at 3:33 AM, Tom Lane wrote: > ... BTW, with respect to the documentation angle, it seems to me > that it'd be better if GenericXLogRegister were renamed to > GenericXLogRegisterBuffer, or perhaps GenericXLogRegisterPage. > I think this would make the

Re: [HACKERS] Some other things about contrib/bloom and generic_xlog.c

2016-04-12 Thread Alexander Korotkov
On Sun, Apr 10, 2016 at 7:49 AM, Tom Lane wrote: > 1. It doesn't seem like generic_xlog.c has thought very carefully about > the semantics of the "hole" between pd_lower and pd_upper. The mainline > XLOG code goes to some lengths to ensure that the hole stays all-zeroes; >

Re: [HACKERS] Some other things about contrib/bloom and generic_xlog.c

2016-04-12 Thread Robert Haas
On Mon, Apr 11, 2016 at 9:54 PM, Craig Ringer wrote: > If you rely on shared_preload_libraries, then "oops, I forgot to put > myextension in shared_preload_libraries on the downstream" becomes a Bad > Thing. There's no way to go back and redo the work you've passed over.

Re: [HACKERS] Some other things about contrib/bloom and generic_xlog.c

2016-04-12 Thread Alexander Korotkov
On Tue, Apr 12, 2016 at 3:08 AM, Michael Paquier wrote: > What I thought we should be able to do with that should not be only > limited to indexes, aka to: > - Be able to register and log full pages > - Be able to log data associated to a block > - Be able to have

Re: [HACKERS] Some other things about contrib/bloom and generic_xlog.c

2016-04-11 Thread Craig Ringer
On 12 April 2016 at 10:09, Michael Paquier wrote: > On Tue, Apr 12, 2016 at 10:54 AM, Craig Ringer > wrote: > > > If you rely on shared_preload_libraries, then "oops, I forgot to put > > myextension in shared_preload_libraries on the

Re: [HACKERS] Some other things about contrib/bloom and generic_xlog.c

2016-04-11 Thread Michael Paquier
On Tue, Apr 12, 2016 at 10:54 AM, Craig Ringer wrote: > On 12 April 2016 at 08:36, Michael Paquier wrote: > The only way we really have at the moment is shared_preload_libraries. That's exactly my point. > If you rely on shared_preload_libraries, then "oops, I forgot to

Re: [HACKERS] Some other things about contrib/bloom and generic_xlog.c

2016-04-11 Thread Craig Ringer
On 12 April 2016 at 08:36, Michael Paquier wrote: > > > Also, the entire point here is that extensions DON'T > > get to have custom apply routines, because we have no way for > > replay to know about such apply routines. (It surely cannot look > > in the catalogs for

Re: [HACKERS] Some other things about contrib/bloom and generic_xlog.c

2016-04-11 Thread Michael Paquier
On Tue, Apr 12, 2016 at 9:33 AM, Tom Lane wrote: > ... BTW, with respect to the documentation angle, it seems to me > that it'd be better if GenericXLogRegister were renamed to > GenericXLogRegisterBuffer, or perhaps GenericXLogRegisterPage. > I think this would make the

Re: [HACKERS] Some other things about contrib/bloom and generic_xlog.c

2016-04-11 Thread Michael Paquier
On Tue, Apr 12, 2016 at 9:21 AM, Tom Lane wrote: > Michael Paquier writes: >> What I thought we should be able to do with that should not be only >> limited to indexes, aka to: >> - Be able to register and log full pages >> - Be able to log data

Re: [HACKERS] Some other things about contrib/bloom and generic_xlog.c

2016-04-11 Thread Tom Lane
... BTW, with respect to the documentation angle, it seems to me that it'd be better if GenericXLogRegister were renamed to GenericXLogRegisterBuffer, or perhaps GenericXLogRegisterPage. I think this would make the documentation clearer, and it would also make it easier to add other sorts of

Re: [HACKERS] Some other things about contrib/bloom and generic_xlog.c

2016-04-11 Thread Tom Lane
Michael Paquier writes: > As given out now, we are able to do the following: > - Log a full page > - Log a delta of a full page, which is actually data associated to a page. > - At recovery, replay those full pages with a delta. Right. > What I thought we should be

Re: [HACKERS] Some other things about contrib/bloom and generic_xlog.c

2016-04-11 Thread Michael Paquier
On Mon, Apr 11, 2016 at 11:29 PM, Tom Lane wrote: > Michael Paquier writes: >> Actually, as I look at this code for the first time, I find that >> GenericXLogFinish() is a very confusing interface. It makes no >> distinction between a page and the

Re: [HACKERS] Some other things about contrib/bloom and generic_xlog.c

2016-04-11 Thread Tom Lane
Teodor Sigaev writes: >> 2. Unless I'm missing something, contrib/bloom is making no effort >> to ensure that bloom index pages can be distinguished from other pages >> by pg_filedump. That's fine if your expectation is that bloom will always >> be a toy with no use in

Re: [HACKERS] Some other things about contrib/bloom and generic_xlog.c

2016-04-11 Thread Tom Lane
Michael Paquier writes: > Actually, as I look at this code for the first time, I find that > GenericXLogFinish() is a very confusing interface. It makes no > distinction between a page and the meta data associated to this page, > this is controlled by a flag isNew and

Re: [HACKERS] Some other things about contrib/bloom and generic_xlog.c

2016-04-11 Thread Alexander Korotkov
Hi, Tom! On Sun, Apr 10, 2016 at 7:49 AM, Tom Lane wrote: > 1. It doesn't seem like generic_xlog.c has thought very carefully about > the semantics of the "hole" between pd_lower and pd_upper. The mainline > XLOG code goes to some lengths to ensure that the hole stays

Re: [HACKERS] Some other things about contrib/bloom and generic_xlog.c

2016-04-11 Thread Teodor Sigaev
mainline XLOG replay. Shouldn't generic_xlog.c take the same trouble? Yes, it should. Alexander now works on it. 2. Unless I'm missing something, contrib/bloom is making no effort to ensure that bloom index pages can be distinguished from other pages by pg_filedump. That's fine if your

Re: [HACKERS] Some other things about contrib/bloom and generic_xlog.c

2016-04-11 Thread Michael Paquier
On Sun, Apr 10, 2016 at 1:49 PM, Tom Lane wrote: > 1. It doesn't seem like generic_xlog.c has thought very carefully about > the semantics of the "hole" between pd_lower and pd_upper. The mainline > XLOG code goes to some lengths to ensure that the hole stays all-zeroes; >

[HACKERS] Some other things about contrib/bloom and generic_xlog.c

2016-04-09 Thread Tom Lane
1. It doesn't seem like generic_xlog.c has thought very carefully about the semantics of the "hole" between pd_lower and pd_upper. The mainline XLOG code goes to some lengths to ensure that the hole stays all-zeroes; for example RestoreBlockImage() explicitly zeroes the hole when restoring from a