[PATCH] Cleanup of GUC units code

Marko Kreen <markokr@gmail.com> writes:

- In attempt to preserve maximum range of values for INT64_IS_BUSTED
systems, the code is written rather non-obvious way.

I do not personally object a bit to making the units comparisons
case-insensitive (I think it's mainly Peter who wants to be strict
about it). I don't think there are any other good ideas in this
patch, however, and exposing ourselves to intermediate overflows in
the name of simplicity is definitely not one.

regards, tom lane

On 9/1/08, Tom Lane <tgl@sss.pgh.pa.us> wrote:

Marko Kreen <markokr@gmail.com> writes:

- In attempt to preserve maximum range of values for INT64_IS_BUSTED
systems, the code is written rather non-obvious way.

I do not personally object a bit to making the units comparisons
case-insensitive (I think it's mainly Peter who wants to be strict
about it). I don't think there are any other good ideas in this
patch, however, and exposing ourselves to intermediate overflows in
the name of simplicity is definitely not one.

For all practical purposes, the overflow is insignificant when int64
works. I'll look if I can avoid it on INT64_IS_BUSTED case.

In the meantime, here is simple patch for case-insensivity.

--
marko

Marko Kreen wrote:

In the meantime, here is simple patch for case-insensivity.

You might be able to talk me into accepting various unambiguous, common
alternative spellings of various units. But for instance allowing MB
and Mb to mean the same thing is insane.

Peter Eisentraut <peter_e@gmx.net> writes:

Marko Kreen wrote:

In the meantime, here is simple patch for case-insensivity.

You might be able to talk me into accepting various unambiguous, common
alternative spellings of various units. But for instance allowing MB and Mb to
mean the same thing is insane.

Because you think some user will be trying to specify their shared_buffers in
bits?

--
Gregory Stark
EnterpriseDB http://www.enterprisedb.com
Ask me about EnterpriseDB's RemoteDBA services!

On 9/2/08, Peter Eisentraut <peter_e@gmx.net> wrote:

Marko Kreen wrote:

In the meantime, here is simple patch for case-insensivity.

You might be able to talk me into accepting various unambiguous, common
alternative spellings of various units. But for instance allowing MB and Mb
to mean the same thing is insane.

How would the docs for that look like? And anyway, what is wrong with
Mb for megabytes? mB may be slightly weird but if some user likes it,
I see no reason to reject it.

You do realize that misspelling unit name can cause downtime of several
minutes instead of couple seconds? We can easily do restart in couple of
seconds but the restart, look logs, launch editor, find value, change,
save, restart cycle will take quite a lot more. Why should we increase
the chance that any config edit causes problems?

Secondly, humans don't have byte-exact memory, instead they generalize
and deduce (eg. from nearby parameters). Thus remembering "KB, MB, GB"
or "kb, mb, gb" is easier than remembering "kB, MB, GB". Also remembering
"ms, s, m, h, d" is easier than "ms, s, min, h, d". (I'm not proposing 'm'
for minutes, just noting, that as we shouldn't ever introduce 'month' unit
for core config values, the 'm' as minutes is preferable for 'min'.)

Thirdly, please don't use "standard units" argument, unless you plan to
propose use of "KiB, MiB, GiB" at the same moment. The units will be
remembered as units-for-postgresql.conf. It is good if they conform to
user expectations, but it's even more important they are easy to remember.

Fourth, for the same reason, it is preferable the amount of units
accepted stays small. Again, to be easily rememberable. This
also kills any chance "mb" can be confused with "milli-bits".

If any extension/module wants to use any other units or full-SI,
it can but that should not extend to core config values. Again,
to reduce chance for confusion.

So, are there any other arguments for keeping current behaviour?

--
marko

Gregory Stark wrote:

Peter Eisentraut <peter_e@gmx.net> writes:

Marko Kreen wrote:

In the meantime, here is simple patch for case-insensivity.

You might be able to talk me into accepting various unambiguous, common
alternative spellings of various units. But for instance allowing MB and Mb to
mean the same thing is insane.

Because you think some user will be trying to specify their shared_buffers in
bits?

My concern is that this information does not stay in the configuration
files. It will invariably leak out into whitepapers, presentations,
product documentation, and before long there will be confusion about why
you can't stuff N Mb over an N Mb connection. I am not making this up.

Mb does not add any typing ease (as "KB" might) or readability (as "sec"
might), and there is no respectable source that will claim it is an
acceptable alias for MB.

On 9/2/08, Peter Eisentraut <peter_e@gmx.net> wrote:

Gregory Stark wrote:

Peter Eisentraut <peter_e@gmx.net> writes:

Marko Kreen wrote:

In the meantime, here is simple patch for case-insensivity.

You might be able to talk me into accepting various unambiguous, common
alternative spellings of various units. But for instance allowing MB

and Mb to

mean the same thing is insane.

Because you think some user will be trying to specify their shared_buffers

in

bits?

My concern is that this information does not stay in the configuration
files. It will invariably leak out into whitepapers, presentations, product
documentation, and before long there will be confusion about why you can't
stuff N Mb over an N Mb connection. I am not making this up.

Uh. So you want force "proper" units in presentations at the price
of everyday admin operations? Does not seem like a sensible tradeoff.

--
marko

"Marko Kreen" <markokr@gmail.com> writes:

Uh. So you want force "proper" units in presentations at the price
of everyday admin operations? Does not seem like a sensible tradeoff.

It didn't to anyone else when Peter wrote the current version either, but as
the person willing to actually do the work and write the code Peter got to
make the decision. Nobody else stepped up to do the work to change it and we
can't exactly force Peter to do work he doesn't agree with. The current
version is unquestionably better than what came before where you had to
specify some things in units of Postgres blocks, for example.

--
Gregory Stark
EnterpriseDB http://www.enterprisedb.com
Ask me about EnterpriseDB's RemoteDBA services!

Gregory Stark <stark@enterprisedb.com> writes:

"Marko Kreen" <markokr@gmail.com> writes:

Uh. So you want force "proper" units in presentations at the price
of everyday admin operations? Does not seem like a sensible tradeoff.

It didn't to anyone else when Peter wrote the current version either, but as
the person willing to actually do the work and write the code Peter got to
make the decision. Nobody else stepped up to do the work to change it and we
can't exactly force Peter to do work he doesn't agree with.

It's not that, in my mind: it's that Peter feels more strongly about it
than the rest of us. This proposal has come up before and he's
successfully argued it down each time. He does have a point about there
being some potential for confusion; and the arguments on the other side
are not much better than "I'm lazy". Being lazy myself, I'd prefer a
case insensitive implementation; but I don't feel strongly enough about
it to want to override Peter's opinion.

regards, tom lane

Anything that will reduce potential downtime should be way to go.
To me it seems that Peter uses the loudest voice and others just don't care
enough.
Using kB for kilobyte seems quite alien and confusing. I have not noticed
that to be used in software i use in my everyday work and could not find any
either with quick search.

On Wed, Sep 3, 2008 at 12:15 AM, Tom Lane <tgl@sss.pgh.pa.us> wrote:

"Asko Oja" <ascoja@gmail.com> writes:

Anything that will reduce potential downtime should be way to go.

That argument didn't seem to me to be worth the electrons to rebut,
but now that someone else has repeated it, maybe I should. It's
ludicrous to claim that allowing case insensitivity here will make
any noticeable improvement in downtime, as there are far more ways
to screw up your config file than this one. (A config file test mode,
such as was discussed a few weeks ago, *would* help, but this won't.)
In the light of Peter's argument about confusion, one could even
argue that admins putting in wrong values because they got confused
about the meaning of the units is a downtime risk in itself.

regards, tom lane

On 9/3/08, Tom Lane <tgl@sss.pgh.pa.us> wrote:

Gregory Stark <stark@enterprisedb.com> writes:

"Marko Kreen" <markokr@gmail.com> writes:

Uh. So you want force "proper" units in presentations at the price
of everyday admin operations? Does not seem like a sensible tradeoff.

It didn't to anyone else when Peter wrote the current version either, but as
the person willing to actually do the work and write the code Peter got to
make the decision. Nobody else stepped up to do the work to change it and we
can't exactly force Peter to do work he doesn't agree with.

It's not that, in my mind: it's that Peter feels more strongly about it
than the rest of us. This proposal has come up before and he's
successfully argued it down each time. He does have a point about there
being some potential for confusion; and the arguments on the other side
are not much better than "I'm lazy". Being lazy myself, I'd prefer a
case insensitive implementation; but I don't feel strongly enough about
it to want to override Peter's opinion.

Um, being lazy is *the point* of units!? And being friendly to
newbies. Can there be any other reason for their existence?

--
marko

On 9/3/08, Tom Lane <tgl@sss.pgh.pa.us> wrote:

"Asko Oja" <ascoja@gmail.com> writes:

Anything that will reduce potential downtime should be way to go.

That argument didn't seem to me to be worth the electrons to rebut,
but now that someone else has repeated it, maybe I should. It's
ludicrous to claim that allowing case insensitivity here will make
any noticeable improvement in downtime, as there are far more ways
to screw up your config file than this one. (A config file test mode,
such as was discussed a few weeks ago, *would* help, but this won't.)
In the light of Peter's argument about confusion, one could even
argue that admins putting in wrong values because they got confused
about the meaning of the units is a downtime risk in itself.

How can the meaning of "gb", "mb", "kb" be uncertain?

But currently writing "kb" *will* load database with wrong values.

--
marko

Marko Kreen wrote:

On 9/2/08, Peter Eisentraut <peter_e@gmx.net> wrote:

Marko Kreen wrote:

In the meantime, here is simple patch for case-insensivity.

You might be able to talk me into accepting various unambiguous, common
alternative spellings of various units. But for instance allowing MB and Mb
to mean the same thing is insane.

How would the docs for that look like? And anyway, what is wrong with
Mb for megabytes?

I doesn't seem completely unreasonable to me that we'd want to express
something in megabits/second in the future. For example, instead of
vacuum_cost_delay, it would be cool to specify a bandwidth allowance.
Megabits/second is a completely reasonable unit for that. Or a limit on
network bandwidth.

FWIW, I don't feel very strongly either way. I'm more than happy with
the status quo. The hint in the error message very clearly spells out
what the valid values are, so it's immediately clear what you need to
fix if you get that wrong.

--
Heikki Linnakangas
EnterpriseDB http://www.enterprisedb.com

On Wed, Sep 3, 2008 at 11:20 AM, Heikki Linnakangas <
heikki.linnakangas@enterprisedb.com> wrote:

Marko Kreen wrote:

On 9/2/08, Peter Eisentraut <peter_e@gmx.net> wrote:

Marko Kreen wrote:

In the meantime, here is simple patch for case-insensivity.

You might be able to talk me into accepting various unambiguous, common
alternative spellings of various units. But for instance allowing MB and
Mb
to mean the same thing is insane.

How would the docs for that look like? And anyway, what is wrong with
Mb for megabytes?

From infamous wikipedia: A *megabit* is a unit of information<http://en.wikipedia.org/wiki/Information&gt;or computer storage, abbreviated

*Mbit* (or *Mb*).
To me playing with case of acronyms and even depending on it seems more
insane. It would make much more sense to have case insensitive set of
acronyms and (thanks Tom for pointing out) some sanity checks when
configuration is loaded to notify user when wrong ones are used for some
context.

I doesn't seem completely unreasonable to me that we'd want to express
something in megabits/second in the future. For example, instead of
vacuum_cost_delay, it would be cool to specify a bandwidth allowance.
Megabits/second is a completely reasonable unit for that. Or a limit on
network bandwidth.

There are less confusing (better) acronyms kbit/s and mbit/s available for
that.

FWIW, I don't feel very strongly either way. I'm more than happy with the
status quo. The hint in the error message very clearly spells out what the
valid values are, so it's immediately clear what you need to fix if you get
that wrong.

Is the database down during that time?

On 9/3/08, Heikki Linnakangas <heikki.linnakangas@enterprisedb.com> wrote:

Marko Kreen wrote:

On 9/2/08, Peter Eisentraut <peter_e@gmx.net> wrote:

Marko Kreen wrote:

In the meantime, here is simple patch for case-insensivity.

You might be able to talk me into accepting various unambiguous, common
alternative spellings of various units. But for instance allowing MB

and Mb

to mean the same thing is insane.

How would the docs for that look like? And anyway, what is wrong with
Mb for megabytes?

I doesn't seem completely unreasonable to me that we'd want to express
something in megabits/second in the future. For example, instead of
vacuum_cost_delay, it would be cool to specify a bandwidth allowance.
Megabits/second is a completely reasonable unit for that. Or a limit on
network bandwidth.

While it sounds theoretically useful, as a UI it's combines worst
from both worlds. You now can confuse user with both case-sensitivity
and unit-mixup. Even if we keep the current case-sensitivity, we should
set policy that units that differ only with case will never be accepted.

And the best way to set the policy in stone would be to make
units case-insensitive.

I like Asko's proposal of 'kbit/s' and 'mbit/s' for those - clear
and no chance of confusion.

FWIW, I don't feel very strongly either way. I'm more than happy with the
status quo. The hint in the error message very clearly spells out what the
valid values are, so it's immediately clear what you need to fix if you get
that wrong.

Well, the problem is that while database may come up with wrong values,
it may be unusable for actual loads, until admin browses the logs,
re-edits the config file and finally restarts in again.

--
marko

On Tue, 2008-09-02 at 16:50 +0300, Peter Eisentraut wrote:

Gregory Stark wrote:

Peter Eisentraut <peter_e@gmx.net> writes:

Marko Kreen wrote:

In the meantime, here is simple patch for case-insensivity.

You might be able to talk me into accepting various unambiguous, common
alternative spellings of various units. But for instance allowing MB and Mb to
mean the same thing is insane.

Because you think some user will be trying to specify their shared_buffers in
bits?

If somebody wants then somebody can still claim that the units are
wrong, as SI specifies 1k = 1000 and 1M=1000000, etc

The binary "should" use IEC multipliers for kibi-, mebi-, tebi-, gibi-,
etc.

http://searchstorage.techtarget.com/sDefinition/0,,sid5_gci825099,00.html

So we could beat "correctness" into DBA-s by forcing them to write KiB
MiB and so on for things that they have always written kb and mb (unless
they are communication engineers, in which case they may be used to
writing these as kB and MB)

My concern is that this information does not stay in the configuration
files. It will invariably leak out into whitepapers, presentations,
product documentation, and before long there will be confusion about why
you can't stuff N Mb over an N Mb connection.

You can't even put N Mbits of data in one sec over N Mbits/sec
connection as there is some overhead from all protocol levels.

Clueless people will also be confused about why they can't fit 1Mbyte of
CVS data into a 1Mbyte database file or why you can't store 1024 MB of
data on a 1GB disk.

I am not making this up.

Is there anything in conf files that can be reasonably specified in
bits ?

Mb does not add any typing ease (as "KB" might) or readability (as "sec"
might), and there is no respectable source that will claim it is an
acceptable alias for MB.

Are you really afraid that someone would want to use mb to mean
millibits ?

As SQL is generally case insensitive, it is quite surprising to most
people that GUC units are not.

-----------------
Hannu

Hannu Krosing wrote:

On Tue, 2008-09-02 at 16:50 +0300, Peter Eisentraut wrote:

Gregory Stark wrote:

Peter Eisentraut <peter_e@gmx.net> writes:

Are you really afraid that someone would want to use mb to mean
millibits ?

As SQL is generally case insensitive, it is quite surprising to most
people that GUC units are not.

We have had this discussion before, I even submitted a patch to make
them case insensitive. In retrospect I was wrong to submit that patch.
SQL may be case insensitive but units are not. MB != Mb != mb , I don't
think we should encourage in any way for users to do the wrong thing.

Sincerely,

Joshua D. Drake

I don't think worrying about the message we send to users is
reasonable. We can take responsibilty for the messages we output but
punishing our users to teach them a lesson is being actively user-
hostile

greg

On 3 Sep 2008, at 15:52, "Joshua D. Drake" <jd@commandprompt.com> wrote: