Обсуждение: Re: [COMMITTERS] pgsql: Move log_newpage and log_newpage_buffer to xlog.c.

On 07/31/2014 05:19 PM, Simon Riggs wrote:
> On 31 July 2014 14:59, Heikki Linnakangas <heikki.linnakangas@iki.fi> wrote:
>
>> Move log_newpage and log_newpage_buffer to xlog.c.
>
> Got a feeling this wasn't properly discussed because its unlikely
> anybody would agree with adding more stuff to xlog.c

Well, they certainly don't belong in heapam.c... Got a better suggestion?

(I bumped into this again while working on the WAL format & API changes, 
which understandably rewrites those functions again. It seemed better to 
do this refactoring separately than include it in the huge patch.)

- Heikki

On 31 July 2014 15:48, Heikki Linnakangas <hlinnaka@iki.fi> wrote:
> On 07/31/2014 05:19 PM, Simon Riggs wrote:
>>
>> On 31 July 2014 14:59, Heikki Linnakangas <heikki.linnakangas@iki.fi>
>> wrote:
>>
>>> Move log_newpage and log_newpage_buffer to xlog.c.
>>
>>
>> Got a feeling this wasn't properly discussed because its unlikely
>> anybody would agree with adding more stuff to xlog.c
>
>
> Well, they certainly don't belong in heapam.c... Got a better suggestion?

I guess if we had xlog_fpi.c with RestoreBackupBlock et all, plus the
stuff you just moved it might make sense.

Just looking for ways to prune it down.

> (I bumped into this again while working on the WAL format & API changes,
> which understandably rewrites those functions again. It seemed better to do
> this refactoring separately than include it in the huge patch.)

Yeh, no problem with the refactoring itself, but I wanted to remind
you to discuss stuff.

-- Simon Riggs                   http://www.2ndQuadrant.com/PostgreSQL Development, 24x7 Support, Training & Services

On 07/31/2014 05:58 PM, Simon Riggs wrote:
> On 31 July 2014 15:48, Heikki Linnakangas <hlinnaka@iki.fi> wrote:
>> On 07/31/2014 05:19 PM, Simon Riggs wrote:
>>>
>>> On 31 July 2014 14:59, Heikki Linnakangas <heikki.linnakangas@iki.fi>
>>> wrote:
>>>
>>>> Move log_newpage and log_newpage_buffer to xlog.c.
>>>
>>> Got a feeling this wasn't properly discussed because its unlikely
>>> anybody would agree with adding more stuff to xlog.c
>>
>> Well, they certainly don't belong in heapam.c... Got a better suggestion?
>
> I guess if we had xlog_fpi.c with RestoreBackupBlock et all, plus the
> stuff you just moved it might make sense.
>
> Just looking for ways to prune it down.

Yeah, something like that might make sense. I totally agree that xlog.c 
is huge (although so is heapam.c). In the WAL format & API patch, I'm 
putting much of the new code for constructing a WAL record into a new 
file, xlogconstruct.c (in the latest version I'm just about to post - 
the previous version put them all in xlog.c).

>> (I bumped into this again while working on the WAL format & API changes,
>> which understandably rewrites those functions again. It seemed better to do
>> this refactoring separately than include it in the huge patch.)
>
> Yeh, no problem with the refactoring itself, but I wanted to remind
> you to discuss stuff.

Ok, thanks. I did post to pgsql-hackers first, see 
http://www.postgresql.org/message-id/53DA0ECF.4040002@vmware.com. 
Admittedly I didn't leave much time for discussion between the post and 
commit; that was because I felt quite certain that people wouldn't 
object to this.

- Heikki

On Thu, Jul 31, 2014 at 11:07 AM, Heikki Linnakangas
<hlinnakangas@vmware.com> wrote:
> Ok, thanks. I did post to pgsql-hackers first, see
> http://www.postgresql.org/message-id/53DA0ECF.4040002@vmware.com. Admittedly
> I didn't leave much time for discussion between the post and commit; that
> was because I felt quite certain that people wouldn't object to this.

I think it's generally a good idea to leave 24 hours anyway, maybe a
bit more over a weekend.

-- 
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company