Thread: Addition of --no-sync to pg_upgrade for test speedup
Hi all, As per $subject, avoiding the flush of the new cluster's data directory shortens a bint the runtime of the test. In some of my slow VMs, aka Windows, this shaves a couple of seconds even if the bulk of the time is still spent on the main regression test suite. In pg_upgrade, we let the flush happen with initdb --sync-only, based on the binary path of the new cluster, so I think that we are not going to miss any test coverage by skipping that. Thoughts or opinions? -- Michael
Attachment
On 16.12.21 07:50, Michael Paquier wrote: > As per $subject, avoiding the flush of the new cluster's data > directory shortens a bint the runtime of the test. In some of my slow > VMs, aka Windows, this shaves a couple of seconds even if the bulk of > the time is still spent on the main regression test suite. > > In pg_upgrade, we let the flush happen with initdb --sync-only, based > on the binary path of the new cluster, so I think that we are not > going to miss any test coverage by skipping that. I think that is reasonable. Maybe we could have some global option, like some environment variable, that enables the "sync" mode in all tests, so it's easy to test that once in a while. Not really a requirement for your patch, but an idea in case this is a concern.
On Fri, Dec 17, 2021 at 10:21:04AM +0100, Peter Eisentraut wrote: > On 16.12.21 07:50, Michael Paquier wrote: > > As per $subject, avoiding the flush of the new cluster's data > > directory shortens a bint the runtime of the test. In some of my slow > > VMs, aka Windows, this shaves a couple of seconds even if the bulk of > > the time is still spent on the main regression test suite. > > > > In pg_upgrade, we let the flush happen with initdb --sync-only, based > > on the binary path of the new cluster, so I think that we are not > > going to miss any test coverage by skipping that. > > I think that is reasonable. > > Maybe we could have some global option, like some environment variable, that > enables the "sync" mode in all tests, so it's easy to test that once in a > while. Not really a requirement for your patch, but an idea in case this is > a concern. Yes, I think it would be good to see all the places we might want to pass the no-sync option. -- Bruce Momjian <bruce@momjian.us> https://momjian.us EDB https://enterprisedb.com If only the physical world exists, free will is an illusion.
On Fri, Dec 17, 2021 at 09:47:05AM -0500, Bruce Momjian wrote: > On Fri, Dec 17, 2021 at 10:21:04AM +0100, Peter Eisentraut wrote: >> I think that is reasonable. Thanks. I have applied that, as that really helped here. >> Maybe we could have some global option, like some environment variable, that >> enables the "sync" mode in all tests, so it's easy to test that once in a >> while. Not really a requirement for your patch, but an idea in case this is >> a concern. > > Yes, I think it would be good to see all the places we might want to > pass the no-sync option. The remaining places in src/bin/ that I can see are pg_resetwal, where we would fsync() a WAL segment full of zeros, and pg_recvlogical OutputFsync(), which does not point to much data, I guess. The first one may be worth it, but that's just 16MB we are talking about and WriteEmptyXLOG() is not a code path taken currently by the tests. We could introduce a new environment variable if one wishes to enforce those flushes, say PG_TEST_SYNC, on top of patching any TAP test that has a --no-sync to filter it out. -- Michael
Attachment
On 2021-Dec-16, Michael Paquier wrote: > In pg_upgrade, we let the flush happen with initdb --sync-only, based > on the binary path of the new cluster, so I think that we are not > going to miss any test coverage by skipping that. There was one patch of mine with breakage that only manifested in the pg_upgrade test *because* of its lack of no-sync. I'm afraid that this change would hide certain problems. https://postgr.es/m/20210130023011.n545o54j65t4kgxn@alap3.anarazel.de > Thoughts or opinions? I'm not 100% comfortable with this. What can we do to preserve *some* testing that include syncing? Maybe some option that a few buildfarm animals use? -- Álvaro Herrera PostgreSQL Developer — https://www.EnterpriseDB.com/ "The Postgresql hackers have what I call a "NASA space shot" mentality. Quite refreshing in a world of "weekend drag racer" developers." (Scott Marlowe)