Ashwani Raina has posted comments on this change. ( http://gerrit.cloudera.org:8080/24963 )
Change subject: [util] an update on --never_fsync flag usage ...................................................................... Patch Set 2: Code-Review+1 (3 comments) Looks good to me. Just a couple nits. Feel free to ignore if you don't see any value in addressing those. http://gerrit.cloudera.org:8080/#/c/24963/2/src/kudu/util/env-test.cc File src/kudu/util/env-test.cc: http://gerrit.cloudera.org:8080/#/c/24963/2/src/kudu/util/env-test.cc@1340 PS2, Line 1340: NSTANTIATE_TEST_SUITE_P(SyncFlags, : FsSyncControlTest, : ::testing::Combine(testing::Bool(), testing::Bool())); nit: to improve readability of the test names, consider adding a name generator lambda instead of having opaque names. Something like this maybe: ++ strings::Substitute("NeverFsync$0_UseFsync$1", std::get<0>(info.param) ? "True" : "False", std::get<1>(info.param) ? "True" : "False"); ++ http://gerrit.cloudera.org:8080/#/c/24963/2/src/kudu/util/env-test.cc@1358 PS2, Line 1358: FLUSH_SYNC nit: here and at line 1376 - consider adding FLUSH_ASYNC variant of tests as well. http://gerrit.cloudera.org:8080/#/c/24963/2/src/kudu/util/env-test.cc@1379 PS2, Line 1379: nit: Maybe add a unit test for SyncDir() as well. -- To view, visit http://gerrit.cloudera.org:8080/24963 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I637e3bd98da98495a38f78176a4a862ab6ae8dd3 Gerrit-Change-Number: 24963 Gerrit-PatchSet: 2 Gerrit-Owner: Alexey Serbin <[email protected]> Gerrit-Reviewer: Alexey Serbin <[email protected]> Gerrit-Reviewer: Ashwani Raina <[email protected]> Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Marton Greber <[email protected]> Gerrit-Comment-Date: Thu, 01 Oct 2026 12:07:21 +0000 Gerrit-HasComments: Yes
