On Thu, Feb 17, 2022 at 2:36 PM Tom Lane <t...@sss.pgh.pa.us> wrote: > Yeah, I came to the same conclusion while out doing some errands. > There's no very convincing reason to believe that what's passed to > progress_update_filename has got adequate lifespan either, or that > that would remain true even if it's true today, or that testing > would detect such a problem (even if we had any, ahem). The file > names I was seeing in testing just now tended to contain fragments > of temporary directory names, which'd be mighty hard to tell from > random garbage in any automated way: > > 3/22774 kB (0%), 0/2 tablespaces (...pc1/PG_15_202202141/5/16392_init) > 3/22774 kB (0%), 1/2 tablespaces (...pc1/PG_15_202202141/5/16392_init) > 22785/22785 kB (100%), 1/2 tablespaces (...t_T8u0/backup1/global/pg_control) > > So I think we should make progress_update_filename strdup each > input while freeing the last value, and insist that every update > go through that logic. (This is kind of a lot of cycles to spend > in support of an optional mode, but maybe we could do it only if > showprogress && verbose.) I'll go make it so.
OK, sounds good, thanks. I couldn't (and still can't) think of a good way of testing the progress-reporting code either. I mean I guess if you could convince pg_basebackup not to truncate the filenames, maybe by convincing it that your terminal is as wide as your garage door, then you could capture the output and do some tests against it. But I feel like the test code would be two orders of magnitude larger than the code it intends to exercise, and I'm not sure it would be entirely robust, either. -- Robert Haas EDB: http://www.enterprisedb.com
