On Wed, Aug 30, 2017 at 4:43 AM, Andres Freund <andres@anarazel.de> wrote:
>
> Hi,
>
> On 2017-08-23 12:13:15 +0530, Beena Emerson wrote:
>> >> + /*
>> >> + * The calculation of XLOGbuffers requires the run-time parameter
>> >> + * XLogSegSize which is set from the control file. This value is
>> >> + * required to create the shared memory segment. Hence, temporarily
>> >> + * allocate space for reading the control file.
>> >> + */
>> >
>> > This makes me uncomfortable. Having to choose the control file multiple
>> > times seems wrong. We're effectively treating the control file as part
>> > of the configuration now, and that means we should move it's parsing to
>> > an earlier part of startup.
>>
>> Yes, this may seem ugly. ControlFile was originally read into the
>> shared memory segment but then we now need the XLogSegSize from the
>> ControlFile to initialise the shared memory segment. I could not
>> figure out any other way to achieve this.
>
> I think reading it one into local memory inside the startup process and
> then copying it into shared memory from there should work?
>.
Done.
>
>> >> @@ -8146,6 +8181,9 @@ InitXLOGAccess(void)
>> >> ThisTimeLineID = XLogCtl->ThisTimeLineID;
>> >> Assert(ThisTimeLineID != 0 || IsBootstrapProcessingMode());
>> >>
>> >> + /* set XLogSegSize */
>> >> + XLogSegSize = ControlFile->xlog_seg_size;
>> >> +
>> >
>> > Hm, why do we have two variables keeping track of the segment size?
>> > wal_segment_size and XLogSegSize? That's bound to lead to confusion.
>> >
>>
>> wal_segment_size is the guc which stores the number of segments
>> (XLogSegSize / XLOG_BLCKSZ).
>
> wal_segment_size and XLogSegSize are the same name, spelt different, so
> if that's where we want to go, we should name them differently. But
> perhaps more fundamentally, I don't see why we need both: What stops us
> from just defining the GUC in bytes?
I made a few changes for this:
- Make XLogSegSize int instead of uint32
- Add a GUC_UNIT_BYT for the unit conversion so that show
wal_segment_size displays user-friendly values.
- track_activity_query_size unit is set to GUC_UNIT_BYT. This was
initially null because we did not have a unit for bytes. This may not
be necessary as it changes the output of SHOW command.
--
Beena Emerson
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers