| Lists: | pgsql-hackers |
|---|
| From: | Heikki Linnakangas <hlinnakangas(at)vmware(dot)com> |
|---|---|
| To: | PostgreSQL-development <pgsql-hackers(at)postgreSQL(dot)org> |
| Subject: | Refactoring GUC unit conversions |
| Date: | 2015-02-13 13:26:14 |
| Message-ID: | 54DDFB76.9080303@vmware.com |
| Views: | Raw Message | Whole Thread | Download mbox | Resend email |
| Lists: | pgsql-hackers |
In the "redesign checkpoint_segments" patch, Robert suggested keeping
the settings' base unit as "number of segments", but allow conversions
from MB, GB etc. I started looking into that and found that adding a new
unit to guc.c is quite messy. The conversions are done with complicated
if-switch-case constructs.
Attached is a patch to refactor that, making the conversions
table-driven. This doesn't change functionality, it just makes the code
nicer.
Any objections?
- Heikki
| Attachment | Content-Type | Size |
|---|---|---|
| 0001-Refactor-unit-conversions-code-in-guc.c.patch | application/x-patch | 13.4 KB |
| From: | Jim Nasby <Jim(dot)Nasby(at)BlueTreble(dot)com> |
|---|---|
| To: | Heikki Linnakangas <hlinnakangas(at)vmware(dot)com>, PostgreSQL-development <pgsql-hackers(at)postgreSQL(dot)org> |
| Subject: | Re: Refactoring GUC unit conversions |
| Date: | 2015-02-13 17:34:36 |
| Message-ID: | 54DE35AC.3080600@BlueTreble.com |
| Views: | Raw Message | Whole Thread | Download mbox | Resend email |
| Lists: | pgsql-hackers |
On 2/13/15 7:26 AM, Heikki Linnakangas wrote:
> In the "redesign checkpoint_segments" patch, Robert suggested keeping
> the settings' base unit as "number of segments", but allow conversions
> from MB, GB etc. I started looking into that and found that adding a new
> unit to guc.c is quite messy. The conversions are done with complicated
> if-switch-case constructs.
>
> Attached is a patch to refactor that, making the conversions
> table-driven. This doesn't change functionality, it just makes the code
> nicer.
Looks good, but shouldn't there be a check for a unit that's neither
memory or time?
--
Jim Nasby, Data Architect, Blue Treble Consulting
Data in Trouble? Get it in Treble! http://BlueTreble.com
| From: | Heikki Linnakangas <hlinnakangas(at)vmware(dot)com> |
|---|---|
| To: | Jim Nasby <Jim(dot)Nasby(at)BlueTreble(dot)com>, PostgreSQL-development <pgsql-hackers(at)postgreSQL(dot)org> |
| Subject: | Re: Refactoring GUC unit conversions |
| Date: | 2015-02-13 17:44:35 |
| Message-ID: | 54DE3803.1010209@vmware.com |
| Views: | Raw Message | Whole Thread | Download mbox | Resend email |
| Lists: | pgsql-hackers |
On 02/13/2015 07:34 PM, Jim Nasby wrote:
> On 2/13/15 7:26 AM, Heikki Linnakangas wrote:
>> In the "redesign checkpoint_segments" patch, Robert suggested keeping
>> the settings' base unit as "number of segments", but allow conversions
>> from MB, GB etc. I started looking into that and found that adding a new
>> unit to guc.c is quite messy. The conversions are done with complicated
>> if-switch-case constructs.
>>
>> Attached is a patch to refactor that, making the conversions
>> table-driven. This doesn't change functionality, it just makes the code
>> nicer.
>
> Looks good, but shouldn't there be a check for a unit that's neither
> memory or time?
Can you elaborate? We currently only support units for memory and time
settings.
- Heikki
| From: | Jim Nasby <Jim(dot)Nasby(at)BlueTreble(dot)com> |
|---|---|
| To: | Heikki Linnakangas <hlinnakangas(at)vmware(dot)com>, PostgreSQL-development <pgsql-hackers(at)postgreSQL(dot)org> |
| Subject: | Re: Refactoring GUC unit conversions |
| Date: | 2015-02-13 20:36:17 |
| Message-ID: | 54DE6041.9030603@BlueTreble.com |
| Views: | Raw Message | Whole Thread | Download mbox | Resend email |
| Lists: | pgsql-hackers |
On 2/13/15 11:44 AM, Heikki Linnakangas wrote:
> On 02/13/2015 07:34 PM, Jim Nasby wrote:
>> On 2/13/15 7:26 AM, Heikki Linnakangas wrote:
>>> In the "redesign checkpoint_segments" patch, Robert suggested keeping
>>> the settings' base unit as "number of segments", but allow conversions
>>> from MB, GB etc. I started looking into that and found that adding a new
>>> unit to guc.c is quite messy. The conversions are done with complicated
>>> if-switch-case constructs.
>>>
>>> Attached is a patch to refactor that, making the conversions
>>> table-driven. This doesn't change functionality, it just makes the code
>>> nicer.
>>
>> Looks good, but shouldn't there be a check for a unit that's neither
>> memory or time?
>
> Can you elaborate? We currently only support units for memory and time
> settings.
I'm thinking an Assert in case someone screws up the function call. But
perhaps I'm just being paranoid.
--
Jim Nasby, Data Architect, Blue Treble Consulting
Data in Trouble? Get it in Treble! http://BlueTreble.com
| From: | Fujii Masao <masao(dot)fujii(at)gmail(dot)com> |
|---|---|
| To: | Heikki Linnakangas <hlinnakangas(at)vmware(dot)com> |
| Cc: | PostgreSQL-development <pgsql-hackers(at)postgresql(dot)org> |
| Subject: | Re: Refactoring GUC unit conversions |
| Date: | 2015-02-26 10:11:30 |
| Message-ID: | CAHGQGwEMKNjKigWA8L5fXoOKz7psEo3q16teAb29MkYuW3nd3A@mail.gmail.com |
| Views: | Raw Message | Whole Thread | Download mbox | Resend email |
| Lists: | pgsql-hackers |
On Fri, Feb 13, 2015 at 10:26 PM, Heikki Linnakangas
<hlinnakangas(at)vmware(dot)com> wrote:
> In the "redesign checkpoint_segments" patch, Robert suggested keeping the
> settings' base unit as "number of segments", but allow conversions from MB,
> GB etc. I started looking into that and found that adding a new unit to
> guc.c is quite messy. The conversions are done with complicated
> if-switch-case constructs.
>
> Attached is a patch to refactor that, making the conversions table-driven.
> This doesn't change functionality, it just makes the code nicer.
Isn't it good idea to allow even wal_keep_segments to converse from MB, GB etc?
Regards,
--
Fujii Masao