Received: from malur.postgresql.org ([217.196.149.56]) by arkaria.postgresql.org with esmtps (TLS1.3:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.92) (envelope-from ) id 1m2ZB3-0006ok-UC for pgsql-hackers@arkaria.postgresql.org; Sun, 11 Jul 2021 13:07:22 +0000 Received: from localhost ([127.0.0.1] helo=malur.postgresql.org) by malur.postgresql.org with esmtp (Exim 4.92) (envelope-from ) id 1m2ZB2-0008UQ-Sa for pgsql-hackers@arkaria.postgresql.org; Sun, 11 Jul 2021 13:07:20 +0000 Received: from makus.postgresql.org ([2001:4800:3e1:1::229]) by malur.postgresql.org with esmtps (TLS1.3:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.92) (envelope-from ) id 1m2ZB2-0008UI-Gq for pgsql-hackers@lists.postgresql.org; Sun, 11 Jul 2021 13:07:20 +0000 Received: from sfr-1.cri.mines-paristech.fr ([77.158.180.225] helo=smtp.cri.ensmp.fr) by makus.postgresql.org with esmtps (TLS1.3:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.92) (envelope-from ) id 1m2ZAz-0005GT-5n for pgsql-hackers@postgresql.org; Sun, 11 Jul 2021 13:07:19 +0000 Received: from pseudo (245.36.4.93.rev.sfr.net [93.4.36.245]) by smtp.cri.ensmp.fr (Postfix) with ESMTPSA id ACF0EC01B9; Sun, 11 Jul 2021 15:09:15 +0200 (CEST) Date: Sun, 11 Jul 2021 15:07:09 +0200 (CEST) From: Fabien COELHO X-X-Sender: fabien@pseudo To: Thomas Munro cc: Michael Paquier , Andrew Dunstan , Alvaro Herrera , Gregory Smith , PostgreSQL-development , david.christensen@crunchydata.com Subject: Re: pgbench logging broken by time logic changes In-Reply-To: Message-ID: References: <202106231308.lxr26cahwxuc@alvherre.pgsql> User-Agent: Alpine 2.22 (DEB 394 2020-01-19) MIME-Version: 1.0 Content-Type: multipart/mixed; boundary="2056645051-1016121140-1626008832=:1240548" List-Id: List-Help: List-Subscribe: List-Post: List-Owner: List-Archive: Archived-At: Precedence: bulk This message is in MIME format. The first part should be readable text, while the remaining parts are likely unreadable without MIME-aware tools. --2056645051-1016121140-1626008832=:1240548 Content-Type: text/plain; format=flowed; charset=US-ASCII Hello Thomas, > I committed the code change without the new TAP tests, because I > didn't want to leave the open item hanging any longer. Ok. Good. > As for the test, ... [...] Argh, so there are no tests that would have caught the regressions:-( > ... I know it can fail, and your v18 didn't fix that, because... > > +check_pgbench_logs($bdir, '001_pgbench_log_1', $nthreads, 1, 3, > ... this range can be exceeded. Indeed. I meant to move that one in the TODO section as well, not just the previous call, so that all time-sensitive tests are fully ignored but reported, which would be enough for me. > I suspect the number of aggregates could be made deterministic, as I > described in an earlier message. What do you think about doing > something like that first for the next release, before trying to add > assertions about the number of aggregates? I think that last time I did something to get more deterministic results in pgbench, which involved a few lines of hocus-pocus in pgbench, the patch got rejected:-) An "ignored" tests looked like a good compromise to check how things are going in the farm and to be able to check for more non regressions when developing pgbench, without introducing behavioral changes. > I'm with you on the importance of testing, but it seems better to start > by making the thing more testable. I'm used to my test patches being rejected, including modifying pgbench behavior to make it more testable. Am I mad enough to retry? Maybe, maybe not. Attached the fully "ignored" version of the time features test as a patch. -- Fabien. --2056645051-1016121140-1626008832=:1240548 Content-Type: text/x-diff; name=pgbench-time-test-1.patch Content-Transfer-Encoding: BASE64 Content-ID: Content-Description: Content-Disposition: attachment; filename=pgbench-time-test-1.patch ZGlmZiAtLWdpdCBhL3NyYy9iaW4vcGdiZW5jaC90LzAwMV9wZ2JlbmNoX3dp dGhfc2VydmVyLnBsIGIvc3JjL2Jpbi9wZ2JlbmNoL3QvMDAxX3BnYmVuY2hf d2l0aF9zZXJ2ZXIucGwNCmluZGV4IDNhYTlkNWQ3NTMuLmYyYTFjODYxYjIg MTAwNjQ0DQotLS0gYS9zcmMvYmluL3BnYmVuY2gvdC8wMDFfcGdiZW5jaF93 aXRoX3NlcnZlci5wbA0KKysrIGIvc3JjL2Jpbi9wZ2JlbmNoL3QvMDAxX3Bn YmVuY2hfd2l0aF9zZXJ2ZXIucGwNCkBAIC04LDYgKzgsNyBAQCB1c2UgUG9z dGdyZXNOb2RlOw0KIHVzZSBUZXN0TGliOw0KIHVzZSBUZXN0OjpNb3JlOw0K IHVzZSBDb25maWc7DQordXNlIFRpbWU6OkhpUmVzIHF3KHRpbWUpOw0KIA0K ICMgc3RhcnQgYSBwZ2JlbmNoIHNwZWNpZmljIHNlcnZlcg0KIG15ICRub2Rl ID0gZ2V0X25ld19ub2RlKCdtYWluJyk7DQpAQCAtNTQsMTIgKzU1LDE0IEBA IHN1YiBwZ2JlbmNoDQogDQogCXB1c2ggQGNtZCwgQGFyZ3M7DQogDQorCW15 ICRzdGFydCA9IHRpbWUoKTsNCiAJJG5vZGUtPmNvbW1hbmRfY2hlY2tzX2Fs bChcQGNtZCwgJHN0YXQsICRvdXQsICRlcnIsICRuYW1lKTsNCisJbXkgJHN0 b3AgPSB0aW1lKCk7DQogDQogCSMgY2xlYW51cD8NCiAJI3VubGluayBAZmls ZW5hbWVzIG9yIGRpZSAiY2Fubm90IHVubGluayBmaWxlcyAoQGZpbGVuYW1l cyk6ICQhIjsNCiANCi0JcmV0dXJuOw0KKwlyZXR1cm4gJHN0b3AgLSAkc3Rh cnQ7DQogfQ0KIA0KICMgdGFibGVzcGFjZSBmb3IgdGVzdGluZywgYmVjYXVz ZSBwYXJ0aXRpb25lZCB0YWJsZXMgY2Fubm90IHVzZSBwZ19kZWZhdWx0DQpA QCAtMTE4Nyw3ICsxMTkwLDcgQEAgc3ViIGNoZWNrX3BnYmVuY2hfbG9ncw0K IA0KIAkjICRwcmVmaXggaXMgc2ltcGxlIGVub3VnaCwgdGh1cyBkb2VzIG5v dCBuZWVkIGVzY2FwaW5nDQogCW15IEBsb2dzID0gbGlzdF9maWxlcygkZGly LCBxcnteJHByZWZpeFwuLiokfSk7DQotCW9rKEBsb2dzID09ICRuYiwgIm51 bWJlciBvZiBsb2cgZmlsZXMiKTsNCisJb2soQGxvZ3MgPT0gJG5iLCAibnVt YmVyIG9mIGxvZyBmaWxlcyAoQGxvZ3MpIik7DQogCW9rKGdyZXAoL1wvJHBy ZWZpeFwuXGQrKFwuXGQrKT8kLywgQGxvZ3MpID09ICRuYiwgImZpbGUgbmFt ZSBmb3JtYXQiKTsNCiANCiAJbXkgJGxvZ19udW1iZXIgPSAwOw0KQEAgLTEy MTksNyArMTIyMiw1OCBAQCBzdWIgY2hlY2tfcGdiZW5jaF9sb2dzDQogDQog bXkgJGJkaXIgPSAkbm9kZS0+YmFzZWRpcjsNCiANCi0jIFJ1biB3aXRoIHNh bXBsaW5nIHJhdGUsIDIgY2xpZW50cyB3aXRoIDUwIHRyYW5zYWN0aW9ucyBl YWNoLg0KK1RPRE86IHsNCisJIw0KKwkjIFRlc3QgdGltZS1zZW5zaXRpdmUg ZmVhdHVyZXMgb24gYSBsaWdodCByZWFkLW9ubHkgdHJhbnNhY3Rpb24NCisJ Iw0KKwlsb2NhbCAkVE9ETyA9ICJwb3NzaWJseSB1bnJlbGlhYmxlIG9uIHNs b3cgaG9zdHMgb3IgdW5sdWNreSBydW5zIjsNCisNCisJIyBSdW4gd2l0aCBz YW1wbGluZyByYXRlLCAyIGNsaWVudHMgd2l0aCA1MCB0cmFuc2FjdGlvbnMg ZWFjaC4NCisJIw0KKwkjICAgLVQ6IGJlbmNoIGR1cmF0aW9uLCAyIHNlY29u ZHMgdG8gZXhlcmNpc2UgcHJvZ3Jlc3MgJiBsb2dzDQorCSMgICAtUDogcHJv Z3Jlc3MgcmVwb3J0DQorCSMgICAtLWFnZ3JlZ2F0ZS1pbnRlcnZhbDogcGVy aW9kaWMgYWdncmVnYXRlZCBsb2dzDQorCSMgICAtLXJhdGU6IHNjaGVkdWxl IGxvYWQNCisJIyAgIC0tbGF0ZW5jeS1saW1pdDogbWF4IGRlbGF5LCBub3Qg ZGVlcGx5IGV4ZXJjaWNlDQorCSMNCisJIyBub3RlOiB0aGUgLS1yYXRlIGJl aGF2aW9yIGlzIHByb2JhYmlsaXN0aWMgaW4gbmF0dXJlLg0KKwkjIG5vdGU6 IC0tcHJvZ3Jlc3MtdGltZXN0YW1wIGlzIG5vdCB0ZXN0ZWQuDQorCW15ICRk ZWxheSA9IHBnYmVuY2goDQorCQknLVQgMiAtUCAxIC1sIC0tYWdncmVnYXRl LWludGVydmFsPTEgLVMgLWIgc2VAMicNCisJCS4gJyAtLXJhdGU9MjAgLS1s YXRlbmN5LWxpbWl0PTEwMDAgLWogJyAuICRudGhyZWFkcw0KKwkJLiAnIC1j IDMgLXInLA0KKwkJMCwNCisJCVsgICBxcnt0eXBlOiBtdWx0aXBsZX0sDQor CQkJcXJ7Y2xpZW50czogM30sDQorCQkJcXJ7dGhyZWFkczogJG50aHJlYWRz fSwNCisJCQlxcntkdXJhdGlvbjogMiBzfSwNCisJCQlxcntzY3JpcHQgMTog Liogc2VsZWN0IG9ubHl9LA0KKwkJCXFye3NjcmlwdCAyOiAuKiBzZWxlY3Qg b25seX0sDQorCQkJcXJ7c3RhdGVtZW50IGxhdGVuY2llcyBpbiBtaWxsaXNl Y29uZHN9LA0KKwkJCXFye0ZST00gcGdiZW5jaF9hY2NvdW50c30gXSwNCisJ CVsgcXJ7dmFjdXVtfSwgcXJ7cHJvZ3Jlc3M6IDFcYn0gXSwNCisJCSdwZ2Jl bmNoIHByb2dyZXNzJywgdW5kZWYsDQorCQkiLS1sb2ctcHJlZml4PSRiZGly LzAwMV9wZ2JlbmNoX2xvZ18xIik7DQorDQorCSMgVGhlIHJhdGUgbWF5IHJl c3VsdHMgaW4gYW4gdW5sdWNreSBzY2hlZHVsZSB3aGljaCB0cmlnZ2Vycw0K KwkjIGFuIGVhcmx5IGV4aXQsIGhlbmNlIHRoZSBsb29zZSBib3VuZC4NCisN CisJIyBhbHNvLCB0aGUgZGVsYXkgbWF5IHRvdGFsbHkgZmFpbCBvbiB2ZXJ5 IHNsb3cgb3Igb3ZlcmxvYWRlZCBob3N0cywNCisJIyB2YWxncmluZCBydW5z Li4uDQorDQorCW9rKDEuNSA8ICRkZWxheSAmJiAkZGVsYXkgPCAyLjUsICIt VCAyIHJ1biBhcm91bmQgMiBzZWNvbmRzIik7DQorDQorCSMgJG50aHJlYWRz IHRocmVhZHMsIDIgc2Vjb25kcywgYnV0IGR1ZSB0byB0aW1pbmcgaW1wcmVj aXNpb24gd2UgbWlnaHQgZ2V0DQorCSMgb25seSAxIG9yIGFzIG1hbnkgYXMg MyBwcm9ncmVzcyByZXBvcnRzIHBlciB0aHJlYWQuDQorCSMgYWdncmVnYXRl IGxvZyBmb3JtYXQgaXM6DQorCSMgICB1bml4X2Vwb2NoX3RpbWUgI3R4IHN1 bSBzdW0yIG1pbiBtYXggW3N1bSBzdW0yIG1pbiBtYXggW3NraXBwZWRdXQ0K KwkjIGZpcnN0IHNlcmllcyBhYm91dCBsYXRlbmN5OyBzZWNvbmQgYWJvdXQg bGFnICgtLXJhdGUpIDsNCisJIyBza2lwcGVkIG9ubHkgaWYgLS1sYXRlbmN5 LWxpbWl0IGlzIHNldC4NCisJY2hlY2tfcGdiZW5jaF9sb2dzKCRiZGlyLCAn MDAxX3BnYmVuY2hfbG9nXzEnLCAkbnRocmVhZHMsIDEsIDMsDQorCQlxcnte XGR7MTAsfSBcZHsxLDJ9IFxkKyBcZCsgXGQrIFxkKyBcZCsgXGQrIFxkKyBc ZCsgXGQrJH0pOw0KK30NCisNCisjIHdpdGggc2FtcGxpbmcgcmF0ZSwgMiBj bGllbnRzIHdpdGggNTAgdHggZWFjaA0KIHBnYmVuY2goDQogCSItbiAtUyAt dCA1MCAtYyAyIC0tbG9nIC0tc2FtcGxpbmctcmF0ZT0wLjUiLCAwLA0KIAlb IHFye3NlbGVjdCBvbmx5fSwgcXJ7cHJvY2Vzc2VkOiAxMDAvMTAwfSBdLCBb cXJ7XiR9XSwNCg== --2056645051-1016121140-1626008832=:1240548--