Horton

Thread

Commits

  1. Rename debug_invalidate_system_caches_always to debug_discard_caches.

  1. "debug_invalidate_system_caches_always" is too long

    Tom Lane <tgl@sss.pgh.pa.us> — 2021-07-04T20:27:13Z

    As I've been poking around in this area, I find myself growing
increasingly annoyed at the new GUC name
"debug_invalidate_system_caches_always".  It is too d*mn long.
It's a serious pain to type in any context where you don't have
autocomplete to help you.  I've kept referring to this type of
testing as CLOBBER_CACHE_ALWAYS testing, even though that name is
now obsolete, just because it's so much shorter.  I think we need
to reconsider this name while we still can.

I do agree with the "debug_" prefix given that it's now visible to
users.  However, it doesn't seem that hard to save some space in
the rest of the name.  The word "system" is adding nothing of value,
and the word "always" seems rather confusing --- if it does
something "always", why is there more than one level?  So a simple
proposal is to rename it to "debug_invalidate_caches".

However, I think we should also give serious consideration to
"debug_clobber_cache" or "debug_clobber_cache_always" for continuity
with past practice (though it still feels like "always" is a good
word to lose now).  "debug_clobber_caches" is another reasonable
variant.

Thoughts?

			regards, tom lane

  2. Re: "debug_invalidate_system_caches_always" is too long

    Noah Misch <noah@leadboat.com> — 2021-07-04T21:12:34Z

    On Sun, Jul 04, 2021 at 04:27:13PM -0400, Tom Lane wrote:
> As I've been poking around in this area, I find myself growing
> increasingly annoyed at the new GUC name
> "debug_invalidate_system_caches_always".  It is too d*mn long.
> It's a serious pain to type in any context where you don't have
> autocomplete to help you.  I've kept referring to this type of
> testing as CLOBBER_CACHE_ALWAYS testing, even though that name is
> now obsolete, just because it's so much shorter.  I think we need
> to reconsider this name while we still can.
> 
> I do agree with the "debug_" prefix given that it's now visible to
> users.  However, it doesn't seem that hard to save some space in
> the rest of the name.  The word "system" is adding nothing of value,
> and the word "always" seems rather confusing --- if it does
> something "always", why is there more than one level?  So a simple
> proposal is to rename it to "debug_invalidate_caches".

I agree with all that.  The word "always" has been misinformation, given the
multiple levels available.

> However, I think we should also give serious consideration to
> "debug_clobber_cache" or "debug_clobber_cache_always" for continuity
> with past practice (though it still feels like "always" is a good
> word to lose now).  "debug_clobber_caches" is another reasonable
> variant.

https://en.wikipedia.org/wiki/Clobbering refers to cases where storage had no
changes to its accessibility but now contains different data.  That doesn't
match InvalidateSystemCaches() especially well, so I think dropping that word
has been a good step.  Some other shorter terms could be debug_flush_caches,
debug_rebuild_caches, or debug_expire_caches.  (debug_caches is tempting, but
that may ensnare folks looking for extra logging rather than a big slowdown.)

  3. Re: "debug_invalidate_system_caches_always" is too long

    Justin Pryzby <pryzby@telsasoft.com> — 2021-07-04T22:22:59Z

    On Sun, Jul 04, 2021 at 04:27:13PM -0400, Tom Lane wrote:
> and the word "always" seems rather confusing --- if it does
> something "always", why is there more than one level?  So a simple
> proposal is to rename it to "debug_invalidate_caches".

+1 to remove "always"

-- 
Justin

  4. Re: "debug_invalidate_system_caches_always" is too long

    Kyotaro Horiguchi <horikyota.ntt@gmail.com> — 2021-07-05T04:52:46Z

    At Sun, 4 Jul 2021 14:12:34 -0700, Noah Misch <noah@leadboat.com> wrote in 
> > However, I think we should also give serious consideration to
> > "debug_clobber_cache" or "debug_clobber_cache_always" for continuity
> > with past practice (though it still feels like "always" is a good
> > word to lose now).  "debug_clobber_caches" is another reasonable
> > variant.
> 
> https://en.wikipedia.org/wiki/Clobbering refers to cases where storage had no
> changes to its accessibility but now contains different data.  That doesn't
> match InvalidateSystemCaches() especially well, so I think dropping that word
> has been a good step.  Some other shorter terms could be debug_flush_caches,
> debug_rebuild_caches, or debug_expire_caches.  (debug_caches is tempting, but

(I murmur that I think "drop" is also usable here.)

> that may ensnare folks looking for extra logging rather than a big slowdown.)

I agree to this.  (And one more +1 to removing "always".)

regards.

-- 
Kyotaro Horiguchi
NTT Open Source Software Center

  5. Re: "debug_invalidate_system_caches_always" is too long

    Amul Sul <sulamul@gmail.com> — 2021-07-05T05:33:13Z

    On Mon, Jul 5, 2021 at 1:57 AM Tom Lane <tgl@sss.pgh.pa.us> wrote:
>
> As I've been poking around in this area, I find myself growing
> increasingly annoyed at the new GUC name
> "debug_invalidate_system_caches_always".  It is too d*mn long.
> It's a serious pain to type in any context where you don't have
> autocomplete to help you.  I've kept referring to this type of
> testing as CLOBBER_CACHE_ALWAYS testing, even though that name is
> now obsolete, just because it's so much shorter.  I think we need
> to reconsider this name while we still can.
>
> I do agree with the "debug_" prefix given that it's now visible to
> users.  However, it doesn't seem that hard to save some space in
> the rest of the name.  The word "system" is adding nothing of value,
> and the word "always" seems rather confusing --- if it does
> something "always", why is there more than one level?  So a simple
> proposal is to rename it to "debug_invalidate_caches".
>
> However, I think we should also give serious consideration to
> "debug_clobber_cache" or "debug_clobber_cache_always" for continuity
> with past practice (though it still feels like "always" is a good
> word to lose now).  "debug_clobber_caches" is another reasonable
> variant.
>
> Thoughts?

 +1 for the "debug_clobber_caches" variant, easy to remember.

Regards,
Amul

  6. Re: "debug_invalidate_system_caches_always" is too long

    Bharath Rupireddy <bharath.rupireddyforpostgres@gmail.com> — 2021-07-05T06:18:17Z

    On Mon, Jul 5, 2021 at 1:57 AM Tom Lane <tgl@sss.pgh.pa.us> wrote:
>
> As I've been poking around in this area, I find myself growing
> increasingly annoyed at the new GUC name
> "debug_invalidate_system_caches_always".  It is too d*mn long.
> It's a serious pain to type in any context where you don't have
> autocomplete to help you.  I've kept referring to this type of
> testing as CLOBBER_CACHE_ALWAYS testing, even though that name is
> now obsolete, just because it's so much shorter.  I think we need
> to reconsider this name while we still can.
>
> I do agree with the "debug_" prefix given that it's now visible to
> users.  However, it doesn't seem that hard to save some space in
> the rest of the name.  The word "system" is adding nothing of value,
> and the word "always" seems rather confusing --- if it does
> something "always", why is there more than one level?  So a simple
> proposal is to rename it to "debug_invalidate_caches".
>
> However, I think we should also give serious consideration to
> "debug_clobber_cache" or "debug_clobber_cache_always" for continuity
> with past practice (though it still feels like "always" is a good
> word to lose now).  "debug_clobber_caches" is another reasonable
> variant.
>
> Thoughts?

+1. IMO, debug_clobber_caches is better because it is simple.  And
also, since the invalidation happens on multiple system caches,
debug_clobber_caches is preferable than debug_clobber_cache.

Regards,
Bharath Rupireddy.

  7. Re: "debug_invalidate_system_caches_always" is too long

    Andrew Dunstan <andrew@dunslane.net> — 2021-07-05T11:25:38Z

    On 7/4/21 4:27 PM, Tom Lane wrote:
> As I've been poking around in this area, I find myself growing
> increasingly annoyed at the new GUC name
> "debug_invalidate_system_caches_always".  It is too d*mn long.
> It's a serious pain to type in any context where you don't have
> autocomplete to help you.  I've kept referring to this type of
> testing as CLOBBER_CACHE_ALWAYS testing, even though that name is
> now obsolete, just because it's so much shorter.  I think we need
> to reconsider this name while we still can.
>
> I do agree with the "debug_" prefix given that it's now visible to
> users.  However, it doesn't seem that hard to save some space in
> the rest of the name.  The word "system" is adding nothing of value,
> and the word "always" seems rather confusing --- if it does
> something "always", why is there more than one level?  So a simple
> proposal is to rename it to "debug_invalidate_caches".
>
> However, I think we should also give serious consideration to
> "debug_clobber_cache" or "debug_clobber_cache_always" for continuity
> with past practice (though it still feels like "always" is a good
> word to lose now).  "debug_clobber_caches" is another reasonable
> variant.
>


+1 for debug_invalidate_caches - it seems to have the most content and
least noise. Second choice would be debug_clobber_caches.


cheers


andrew


--
Andrew Dunstan
EDB: https://www.enterprisedb.com

  8. Re: "debug_invalidate_system_caches_always" is too long

    Tom Lane <tgl@sss.pgh.pa.us> — 2021-07-05T19:13:41Z

    Noah Misch <noah@leadboat.com> writes:
> On Sun, Jul 04, 2021 at 04:27:13PM -0400, Tom Lane wrote:
>> However, I think we should also give serious consideration to
>> "debug_clobber_cache" or "debug_clobber_cache_always" for continuity
>> with past practice (though it still feels like "always" is a good
>> word to lose now).  "debug_clobber_caches" is another reasonable
>> variant.

> https://en.wikipedia.org/wiki/Clobbering refers to cases where storage had no
> changes to its accessibility but now contains different data.  That doesn't
> match InvalidateSystemCaches() especially well, so I think dropping that word
> has been a good step.  Some other shorter terms could be debug_flush_caches,
> debug_rebuild_caches, or debug_expire_caches.  (debug_caches is tempting, but
> that may ensnare folks looking for extra logging rather than a big slowdown.)

I like "debug_flush_caches" --- it's short and accurate.

			regards, tom lane

  9. Re: "debug_invalidate_system_caches_always" is too long

    Bharath Rupireddy <bharath.rupireddyforpostgres@gmail.com> — 2021-07-06T03:46:54Z

    On Tue, Jul 6, 2021 at 12:43 AM Tom Lane <tgl@sss.pgh.pa.us> wrote:
>
> Noah Misch <noah@leadboat.com> writes:
> > On Sun, Jul 04, 2021 at 04:27:13PM -0400, Tom Lane wrote:
> >> However, I think we should also give serious consideration to
> >> "debug_clobber_cache" or "debug_clobber_cache_always" for continuity
> >> with past practice (though it still feels like "always" is a good
> >> word to lose now).  "debug_clobber_caches" is another reasonable
> >> variant.
>
> > https://en.wikipedia.org/wiki/Clobbering refers to cases where storage had no
> > changes to its accessibility but now contains different data.  That doesn't
> > match InvalidateSystemCaches() especially well, so I think dropping that word
> > has been a good step.  Some other shorter terms could be debug_flush_caches,
> > debug_rebuild_caches, or debug_expire_caches.  (debug_caches is tempting, but
> > that may ensnare folks looking for extra logging rather than a big slowdown.)
>
> I like "debug_flush_caches" --- it's short and accurate.

Do we always flush the cache entries into the disk? Sometimes we just
invalidate the cache entries in the registered invalidation callbacks,
right? Since we already use the term "clobber" in the user visible
config option --clobber-cache, isn't it consistent to use
debug_clobber_caches?

Regards,
Bharath Rupireddy.

  10. Re: "debug_invalidate_system_caches_always" is too long

    Andrew Dunstan <andrew@dunslane.net> — 2021-07-06T09:50:14Z

    On 7/5/21 11:46 PM, Bharath Rupireddy wrote:
> On Tue, Jul 6, 2021 at 12:43 AM Tom Lane <tgl@sss.pgh.pa.us> wrote:
>> Noah Misch <noah@leadboat.com> writes:
>>> On Sun, Jul 04, 2021 at 04:27:13PM -0400, Tom Lane wrote:
>>>> However, I think we should also give serious consideration to
>>>> "debug_clobber_cache" or "debug_clobber_cache_always" for continuity
>>>> with past practice (though it still feels like "always" is a good
>>>> word to lose now).  "debug_clobber_caches" is another reasonable
>>>> variant.
>>> https://en.wikipedia.org/wiki/Clobbering refers to cases where storage had no
>>> changes to its accessibility but now contains different data.  That doesn't
>>> match InvalidateSystemCaches() especially well, so I think dropping that word
>>> has been a good step.  Some other shorter terms could be debug_flush_caches,
>>> debug_rebuild_caches, or debug_expire_caches.  (debug_caches is tempting, but
>>> that may ensnare folks looking for extra logging rather than a big slowdown.)
>> I like "debug_flush_caches" --- it's short and accurate.
> Do we always flush the cache entries into the disk? Sometimes we just
> invalidate the cache entries in the registered invalidation callbacks,
> right? Since we already use the term "clobber" in the user visible
> config option --clobber-cache, isn't it consistent to use
> debug_clobber_caches?
>

I think 'flush' here means simply 'discard'. Maybe that would be a
better word to use.


cheers


andrew

--
Andrew Dunstan
EDB: https://www.enterprisedb.com

  11. Re: "debug_invalidate_system_caches_always" is too long

    Tom Lane <tgl@sss.pgh.pa.us> — 2021-07-06T13:24:28Z

    Andrew Dunstan <andrew@dunslane.net> writes:
> On 7/5/21 11:46 PM, Bharath Rupireddy wrote:
>> On Tue, Jul 6, 2021 at 12:43 AM Tom Lane <tgl@sss.pgh.pa.us> wrote:
>>> I like "debug_flush_caches" --- it's short and accurate.

>> Do we always flush the cache entries into the disk? Sometimes we just
>> invalidate the cache entries in the registered invalidation callbacks,
>> right? Since we already use the term "clobber" in the user visible
>> config option --clobber-cache, isn't it consistent to use
>> debug_clobber_caches?

> I think 'flush' here means simply 'discard'. Maybe that would be a
> better word to use.

"Discard" could be misinterpreted too, no doubt.  None of these words
have one single exact meaning, so I have only limited patience for
this sort of argumentation.

(As for initdb's "--clobber-cache", I'm assuming we'd rename that to
match whatever we come up with here.  It is what it is now only because
I was unwilling to call it "--use-debug-invalidate-system-caches-always".)

			regards, tom lane

  12. Re: "debug_invalidate_system_caches_always" is too long

    Peter Eisentraut <peter.eisentraut@enterprisedb.com> — 2021-07-07T08:37:15Z

    On 04.07.21 22:27, Tom Lane wrote:
> I do agree with the "debug_" prefix given that it's now visible to
> users.  However, it doesn't seem that hard to save some space in
> the rest of the name.  The word "system" is adding nothing of value,
> and the word "always" seems rather confusing --- if it does
> something "always", why is there more than one level?  So a simple
> proposal is to rename it to "debug_invalidate_caches".

I think we can definitely drop the "always".  Not so much the "system", 
since there are other caches, but it would be ok if we want it shorter.

> However, I think we should also give serious consideration to
> "debug_clobber_cache" or "debug_clobber_cache_always" for continuity
> with past practice (though it still feels like "always" is a good
> word to lose now).  "debug_clobber_caches" is another reasonable
> variant.

The clobbering doesn't actually happen unless you turn on 
CLOBBER_FREED_MEMORY, so it would be good to keep that separate.

  13. Re: "debug_invalidate_system_caches_always" is too long

    Tom Lane <tgl@sss.pgh.pa.us> — 2021-07-07T15:16:57Z

    Peter Eisentraut <peter.eisentraut@enterprisedb.com> writes:
> The clobbering doesn't actually happen unless you turn on 
> CLOBBER_FREED_MEMORY, so it would be good to keep that separate.

Fair point.  What do you think of the alternative proposals
"debug_flush_caches", "debug_discard_caches", etc?

			regards, tom lane

  14. Re: "debug_invalidate_system_caches_always" is too long

    Robert Haas <robertmhaas@gmail.com> — 2021-07-08T17:33:28Z

    On Wed, Jul 7, 2021 at 11:17 AM Tom Lane <tgl@sss.pgh.pa.us> wrote:
> Peter Eisentraut <peter.eisentraut@enterprisedb.com> writes:
> > The clobbering doesn't actually happen unless you turn on
> > CLOBBER_FREED_MEMORY, so it would be good to keep that separate.
>
> Fair point.  What do you think of the alternative proposals
> "debug_flush_caches", "debug_discard_caches", etc?

I like debug_discard_caches best. I have no preference between
debug_flush_caches and debug_clobber_caches; neither seems horrid. I
agree that what we're doing here is not precisely a "clobber" in the
usual sense, but the people who are apt to be using it will probably
be aware of that. Yet, it's good to try to clear things up for future
hackers, and IMHO debug_discard_caches is the clearest, so that's why
I like it a little better than the other choices.

-- 
Robert Haas
EDB: http://www.enterprisedb.com

  15. Re: "debug_invalidate_system_caches_always" is too long

    Tom Lane <tgl@sss.pgh.pa.us> — 2021-07-08T20:34:55Z

    Robert Haas <robertmhaas@gmail.com> writes:
> On Wed, Jul 7, 2021 at 11:17 AM Tom Lane <tgl@sss.pgh.pa.us> wrote:
>> Fair point.  What do you think of the alternative proposals
>> "debug_flush_caches", "debug_discard_caches", etc?

> I like debug_discard_caches best.

I can live with that.  Anyone strongly against it?

			regards, tom lane

  16. Re: "debug_invalidate_system_caches_always" is too long

    Alvaro Herrera <alvherre@alvh.no-ip.org> — 2021-07-08T20:59:03Z

    On 2021-Jul-08, Tom Lane wrote:

> Robert Haas <robertmhaas@gmail.com> writes:
> > On Wed, Jul 7, 2021 at 11:17 AM Tom Lane <tgl@sss.pgh.pa.us> wrote:
> >> Fair point.  What do you think of the alternative proposals
> >> "debug_flush_caches", "debug_discard_caches", etc?
> 
> > I like debug_discard_caches best.
> 
> I can live with that.  Anyone strongly against it?

Seems fine to me.

-- 
Álvaro Herrera           39°49'30"S 73°17'W  —  https://www.EnterpriseDB.com/
"Just treat us the way you want to be treated + some extra allowance
 for ignorance."                                    (Michael Brusser)

  17. Re: "debug_invalidate_system_caches_always" is too long

    Noah Misch <noah@leadboat.com> — 2021-07-09T11:12:11Z

    On Thu, Jul 08, 2021 at 04:34:55PM -0400, Tom Lane wrote:
> Robert Haas <robertmhaas@gmail.com> writes:
> > On Wed, Jul 7, 2021 at 11:17 AM Tom Lane <tgl@sss.pgh.pa.us> wrote:
> >> Fair point.  What do you think of the alternative proposals
> >> "debug_flush_caches", "debug_discard_caches", etc?
> 
> > I like debug_discard_caches best.
> 
> I can live with that.  Anyone strongly against it?

I like it.

  18. Re: "debug_invalidate_system_caches_always" is too long

    Tom Lane <tgl@sss.pgh.pa.us> — 2021-07-12T18:49:12Z

    Noah Misch <noah@leadboat.com> writes:
> On Thu, Jul 08, 2021 at 04:34:55PM -0400, Tom Lane wrote:
>> Robert Haas <robertmhaas@gmail.com> writes:
>>> I like debug_discard_caches best.

>> I can live with that.  Anyone strongly against it?

> I like it.

Hearing no votes against, here's a proposed patch for that.

(This is for HEAD; I expect v14 will need additional adjustments
in release-14.sgml)

			regards, tom lane