From: | Rafia Sabih <rafia(dot)sabih(at)enterprisedb(dot)com> |
---|---|
To: | Robert Haas <robertmhaas(at)gmail(dot)com> |
Cc: | Mithun Cy <mithun(dot)cy(at)enterprisedb(dot)com>, Andres Freund <andres(at)anarazel(dot)de>, Peter Eisentraut <peter(dot)eisentraut(at)2ndquadrant(dot)com>, Beena Emerson <memissemerson(at)gmail(dot)com>, Jim Nasby <Jim(dot)Nasby(at)bluetreble(dot)com>, Amit Kapila <amit(dot)kapila16(at)gmail(dot)com>, Michael Paquier <michael(dot)paquier(at)gmail(dot)com>, pgsql-hackers <pgsql-hackers(at)postgresql(dot)org> |
Subject: | Re: Proposal : For Auto-Prewarm. |
Date: | 2017-06-06 10:29:30 |
Message-ID: | CAOGQiiMX8NrPLRN1KyOM-yOzXw5UA8c3kaeQLFJNehknWOf0og@mail.gmail.com |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
On Mon, Jun 5, 2017 at 8:06 PM, Robert Haas <robertmhaas(at)gmail(dot)com> wrote:
>
> Many of these seem worse, like these ones:
>
> - * Quit if we've reached records for another database. Unless the
> + * Quit if we've reached records of another database. Unless the
>
Why is it worse? I've always encountered using "records of database"
and not "records for database". Anyhow, I tried reframing the sentence
altogether.
> - * When we reach a new relation, close the old one. Note, however,
> - * that the previous try_relation_open may have failed, in which case
> - * rel will be NULL.
> + * On reaching a new relation, close the old one. Note, that the
> + * previous try_relation_open may have failed, in which case rel will
> + * be NULL.
>
Reframed the sentence.
> - * Try to open each new relation, but only once, when we first
> - * encounter it. If it's been dropped, skip the associated blocks.
> + * Each relation is open only once at it's first encounter. If it's
> + * been dropped, skip the associated blocks.
>
Reframed.
> IMHO, there's still a good bit of work needed here to make this sound
> like American English. For example:
>
> - * It is a bgworker which automatically records information about blocks
> - * which were present in buffer pool before server shutdown and then
> - * prewarm the buffer pool upon server restart with those blocks.
> + * It is a bgworker process that automatically records information about
> + * blocks which were present in buffer pool before server
> shutdown and then
> + * prewarms the buffer pool upon server restart with those blocks.
>
> This construction "It is a..." without a clear referent seems to be
> standard in Indian English, but it looks wrong to English speakers
> from other parts of the world, or at least to me.
>
Agreed, tried reframing the sentence.
> + * Since there could be at max one worker who could do a prewarm, hence,
> + * acquiring locks is not required before setting skip_prewarm_on_restart.
>
> To me, adding a comma before hence looks like a significant
> improvement, but the word hence itself seems out-of-place. Also, I'd
> change "at max" to "at most" and maybe reword the sentence a little.
> There's a lot of little things like this which I have tended be quite
> strict about changing before commit; I occasionally wonder whether
> it's really worth the effort. It's not really wrong, it just sounds
> weird to me as an American.
>
Agree, sentence reframed.
I am attaching the patch with the modifications I made on a second look.
--
Regards,
Rafia Sabih
EnterpriseDB: http://www.enterprisedb.com/
Attachment | Content-Type | Size |
---|---|---|
cosmetic_autoprewarm_v2.patch | application/octet-stream | 22.0 KB |
From | Date | Subject | |
---|---|---|---|
Next Message | Petr Jelinek | 2017-06-06 10:51:17 | Re: inconsistent application_name use in logical workers |
Previous Message | Michael Meskes | 2017-06-06 10:20:19 | Re: Is ECPG's SET CONNECTION really not thread-aware? |