Re: "debug_invalidate_system_caches_always" is too long
Noah Misch writes: > On Thu, Jul 08, 2021 at 04:34:55PM -0400, Tom Lane wrote: >> Robert Haas 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 diff --git a/contrib/postgres_fdw/expected/postgres_fdw.out b/contrib/postgres_fdw/expected/postgres_fdw.out index b8561d6a3c..e5bbf3b0af 100644 --- a/contrib/postgres_fdw/expected/postgres_fdw.out +++ b/contrib/postgres_fdw/expected/postgres_fdw.out @@ -9340,11 +9340,11 @@ WARNING: there is no transaction in progress -- Change application_name of remote connection to special one -- so that we can easily terminate the connection later. ALTER SERVER loopback OPTIONS (application_name 'fdw_retry_check'); --- If debug_invalidate_system_caches_always is active, it results in +-- If debug_discard_caches is active, it results in -- dropping remote connections after every transaction, making it -- impossible to test termination meaningfully. So turn that off -- for this test. -SET debug_invalidate_system_caches_always = 0; +SET debug_discard_caches = 0; -- Make sure we have a remote connection. SELECT 1 FROM ft1 LIMIT 1; ?column? @@ -9386,7 +9386,7 @@ SELECT 1 FROM ft1 LIMIT 1;-- should fail ERROR: 08006 \set VERBOSITY default COMMIT; -RESET debug_invalidate_system_caches_always; +RESET debug_discard_caches; -- = -- test connection invalidation cases and postgres_fdw_get_connections function -- = diff --git a/contrib/postgres_fdw/sql/postgres_fdw.sql b/contrib/postgres_fdw/sql/postgres_fdw.sql index c283e74715..fe503ed6c3 100644 --- a/contrib/postgres_fdw/sql/postgres_fdw.sql +++ b/contrib/postgres_fdw/sql/postgres_fdw.sql @@ -2831,11 +2831,11 @@ ROLLBACK; -- so that we can easily terminate the connection later. ALTER SERVER loopback OPTIONS (application_name 'fdw_retry_check'); --- If debug_invalidate_system_caches_always is active, it results in +-- If debug_discard_caches is active, it results in -- dropping remote connections after every transaction, making it -- impossible to test termination meaningfully. So turn that off -- for this test. -SET debug_invalidate_system_caches_always = 0; +SET debug_discard_caches = 0; -- Make sure we have a remote connection. SELECT 1 FROM ft1 LIMIT 1; @@ -2861,7 +2861,7 @@ SELECT 1 FROM ft1 LIMIT 1;-- should fail \set VERBOSITY default COMMIT; -RESET debug_invalidate_system_caches_always; +RESET debug_discard_caches; -- = -- test connection invalidation cases and postgres_fdw_get_connections function diff --git a/doc/src/sgml/config.sgml b/doc/src/sgml/config.sgml index 381d8636ab..d1b889b80f 100644 --- a/doc/src/sgml/config.sgml +++ b/doc/src/sgml/config.sgml @@ -10341,10 +10341,10 @@ dynamic_library_path = 'C:\tools\postgresql;H:\my_project\lib;$libdir' - - debug_invalidate_system_caches_always (integer) + + debug_discard_caches (integer) - debug_invalidate_system_caches_always configuration parameter + debug_discard_caches configuration parameter @@ -10369,7 +10369,7 @@ dynamic_library_path = 'C:\tools\postgresql;H:\my_project\lib;$libdir' This parameter is supported when -CLOBBER_CACHE_ENABLED was defined at compile time +DISCARD_CACHES_ENABLED was defined at compile time (which happens automatically when using the configure option --enable-cassert). In production builds, its value diff --git a/doc/src/sgml/ref/initdb.sgml b/doc/src/sgml/ref/initdb.sgml index 3077530c7b..e62742850a 100644 --- a/doc/src/sgml/ref/initdb.sgml +++ b/doc/src/sgml/ref/initdb.sgml @@ -388,17 +388,6 @@ PostgreSQL documentation Other, less commonly used, options are also available: - - --clobber-cache - - -Run the bootstrap backend with the -debug_invalidate_system_caches_always=1 option. -This takes a very long time and is only of use for deep debugging. - - - - -d --debug @@ -413,6 +402,17 @@ PostgreSQL documentation + + --discard-caches + + +Run the bootstrap backend with the +debug_discard_caches=1 option. +This takes a very long time and is only of use for deep debugging. + + + + -L directory diff --git a/doc/src/sgml/regress.sgml b/doc/src/sgml/regress.sgml index c35e036028..acc7a50c2f 100644 --- a/doc/src/sgml/regress.sgml +++ b/doc/src/sgml/regress.sgml @@ -373,7 +373,7 @@ make
Re: "debug_invalidate_system_caches_always" is too long
On Thu, Jul 08, 2021 at 04:34:55PM -0400, Tom Lane wrote: > Robert Haas writes: > > On Wed, Jul 7, 2021 at 11:17 AM Tom Lane 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.
Re: "debug_invalidate_system_caches_always" is too long
On 2021-Jul-08, Tom Lane wrote: > Robert Haas writes: > > On Wed, Jul 7, 2021 at 11:17 AM Tom Lane 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)
Re: "debug_invalidate_system_caches_always" is too long
Robert Haas writes: > On Wed, Jul 7, 2021 at 11:17 AM Tom Lane 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
Re: "debug_invalidate_system_caches_always" is too long
On Wed, Jul 7, 2021 at 11:17 AM Tom Lane wrote: > Peter Eisentraut 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
Re: "debug_invalidate_system_caches_always" is too long
Peter Eisentraut 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
Re: "debug_invalidate_system_caches_always" is too long
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.
Re: "debug_invalidate_system_caches_always" is too long
Andrew Dunstan writes: > On 7/5/21 11:46 PM, Bharath Rupireddy wrote: >> On Tue, Jul 6, 2021 at 12:43 AM Tom Lane 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
Re: "debug_invalidate_system_caches_always" is too long
On 7/5/21 11:46 PM, Bharath Rupireddy wrote: > On Tue, Jul 6, 2021 at 12:43 AM Tom Lane wrote: >> Noah Misch 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
Re: "debug_invalidate_system_caches_always" is too long
On Tue, Jul 6, 2021 at 12:43 AM Tom Lane wrote: > > Noah Misch 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.
Re: "debug_invalidate_system_caches_always" is too long
Noah Misch 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
Re: "debug_invalidate_system_caches_always" is too long
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
Re: "debug_invalidate_system_caches_always" is too long
On Mon, Jul 5, 2021 at 1:57 AM 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. > > 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.
Re: "debug_invalidate_system_caches_always" is too long
On Mon, Jul 5, 2021 at 1:57 AM 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. > > Thoughts? +1 for the "debug_clobber_caches" variant, easy to remember. Regards, Amul
Re: "debug_invalidate_system_caches_always" is too long
At Sun, 4 Jul 2021 14:12:34 -0700, Noah Misch 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
Re: "debug_invalidate_system_caches_always" is too long
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
Re: "debug_invalidate_system_caches_always" is too long
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.)
"debug_invalidate_system_caches_always" is too long
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
