Use SpecialEnv + smaller testing db to speed up some slow BackupEngineRateLimitingTestWithParam #9974

hx235 · 2022-05-10T04:04:19Z

Context:
BackupEngineRateLimitingTestWithParam.RateLimiting and BackupEngineRateLimitingTestWithParam.RateLimitingVerifyBackup involve creating backup and restoring of a big database with rate-limiting. Using the normal env with a normal clock requires real elapse of time (13702 - 19848 ms/per test). As suggested in #8722 (comment), this PR is to speed it up with SpecialEnv (time_elapse_only_sleep=true) where its clock accepts fake elapse of time during rate-limiting and by using a smaller db. This results in -90% in time (100 - 600 ms/per test)

Summary:

Added TEST_ function to set clock of the default rate limiters in backup engine
Shrunk testdb by 10 times while keeping it big enough for testing
Renamed some test variables and reorganized some if-else branch for clarity without changing the test

Test plan:

Run tests pre/post PR the same time to verify the tests are sped up by 90 - 95%
BackupEngineRateLimitingTestWithParam.RateLimiting
Pre:

[ RUN      ] RateLimiting/BackupEngineRateLimitingTestWithParam.RateLimiting/0
[       OK ] RateLimiting/BackupEngineRateLimitingTestWithParam.RateLimiting/0 (11123 ms)
[ RUN      ] RateLimiting/BackupEngineRateLimitingTestWithParam.RateLimiting/1
[       OK ] RateLimiting/BackupEngineRateLimitingTestWithParam.RateLimiting/1 (9441 ms)
[ RUN      ] RateLimiting/BackupEngineRateLimitingTestWithParam.RateLimiting/2
[       OK ] RateLimiting/BackupEngineRateLimitingTestWithParam.RateLimiting/2 (11096 ms)
[ RUN      ] RateLimiting/BackupEngineRateLimitingTestWithParam.RateLimiting/3
[       OK ] RateLimiting/BackupEngineRateLimitingTestWithParam.RateLimiting/3 (9339 ms)
[ RUN      ] RateLimiting/BackupEngineRateLimitingTestWithParam.RateLimiting/4
[       OK ] RateLimiting/BackupEngineRateLimitingTestWithParam.RateLimiting/4 (11121 ms)
[ RUN      ] RateLimiting/BackupEngineRateLimitingTestWithParam.RateLimiting/5
[       OK ] RateLimiting/BackupEngineRateLimitingTestWithParam.RateLimiting/5 (9413 ms)
[ RUN      ] RateLimiting/BackupEngineRateLimitingTestWithParam.RateLimiting/6
[       OK ] RateLimiting/BackupEngineRateLimitingTestWithParam.RateLimiting/6 (11185 ms)
[ RUN      ] RateLimiting/BackupEngineRateLimitingTestWithParam.RateLimiting/7
[       OK ] RateLimiting/BackupEngineRateLimitingTestWithParam.RateLimiting/7 (9511 ms)
[----------] 8 tests from RateLimiting/BackupEngineRateLimitingTestWithParam (82230 ms total)

Post:

[ RUN      ] RateLimiting/BackupEngineRateLimitingTestWithParam.RateLimiting/0
[       OK ] RateLimiting/BackupEngineRateLimitingTestWithParam.RateLimiting/0 (395 ms)
[ RUN      ] RateLimiting/BackupEngineRateLimitingTestWithParam.RateLimiting/1
[       OK ] RateLimiting/BackupEngineRateLimitingTestWithParam.RateLimiting/1 (564 ms)
[ RUN      ] RateLimiting/BackupEngineRateLimitingTestWithParam.RateLimiting/2
[       OK ] RateLimiting/BackupEngineRateLimitingTestWithParam.RateLimiting/2 (358 ms)
[ RUN      ] RateLimiting/BackupEngineRateLimitingTestWithParam.RateLimiting/3
[       OK ] RateLimiting/BackupEngineRateLimitingTestWithParam.RateLimiting/3 (567 ms)
[ RUN      ] RateLimiting/BackupEngineRateLimitingTestWithParam.RateLimiting/4
[       OK ] RateLimiting/BackupEngineRateLimitingTestWithParam.RateLimiting/4 (173 ms)
[ RUN      ] RateLimiting/BackupEngineRateLimitingTestWithParam.RateLimiting/5
[       OK ] RateLimiting/BackupEngineRateLimitingTestWithParam.RateLimiting/5 (176 ms)
[ RUN      ] RateLimiting/BackupEngineRateLimitingTestWithParam.RateLimiting/6
[       OK ] RateLimiting/BackupEngineRateLimitingTestWithParam.RateLimiting/6 (191 ms)
[ RUN      ] RateLimiting/BackupEngineRateLimitingTestWithParam.RateLimiting/7
[       OK ] RateLimiting/BackupEngineRateLimitingTestWithParam.RateLimiting/7 (177 ms)
[----------] 8 tests from RateLimiting/BackupEngineRateLimitingTestWithParam (2601 ms total)

BackupEngineRateLimitingTestWithParam.RateLimitingVerifyBackup
Pre:

[ RUN      ] RateLimiting/BackupEngineRateLimitingTestWithParam.RateLimitingVerifyBackup/0
[       OK ] RateLimiting/BackupEngineRateLimitingTestWithParam.RateLimitingVerifyBackup/0 (7275 ms)
[ RUN      ] RateLimiting/BackupEngineRateLimitingTestWithParam.RateLimitingVerifyBackup/1
[       OK ] RateLimiting/BackupEngineRateLimitingTestWithParam.RateLimitingVerifyBackup/1 (3961 ms)
[ RUN      ] RateLimiting/BackupEngineRateLimitingTestWithParam.RateLimitingVerifyBackup/2
[       OK ] RateLimiting/BackupEngineRateLimitingTestWithParam.RateLimitingVerifyBackup/2 (7117 ms)
[ RUN      ] RateLimiting/BackupEngineRateLimitingTestWithParam.RateLimitingVerifyBackup/3
[       OK ] RateLimiting/BackupEngineRateLimitingTestWithParam.RateLimitingVerifyBackup/3 (3921 ms)
[ RUN      ] RateLimiting/BackupEngineRateLimitingTestWithParam.RateLimitingVerifyBackup/4
[       OK ] RateLimiting/BackupEngineRateLimitingTestWithParam.RateLimitingVerifyBackup/4 (19862 ms)
[ RUN      ] RateLimiting/BackupEngineRateLimitingTestWithParam.RateLimitingVerifyBackup/5
[       OK ] RateLimiting/BackupEngineRateLimitingTestWithParam.RateLimitingVerifyBackup/5 (10231 ms)
[ RUN      ] RateLimiting/BackupEngineRateLimitingTestWithParam.RateLimitingVerifyBackup/6
[       OK ] RateLimiting/BackupEngineRateLimitingTestWithParam.RateLimitingVerifyBackup/6 (19848 ms)
[ RUN      ] RateLimiting/BackupEngineRateLimitingTestWithParam.RateLimitingVerifyBackup/7
[       OK ] RateLimiting/BackupEngineRateLimitingTestWithParam.RateLimitingVerifyBackup/7 (10372 ms)
[----------] 8 tests from RateLimiting/BackupEngineRateLimitingTestWithParam (82587 ms total)

Post:

[ RUN      ] RateLimiting/BackupEngineRateLimitingTestWithParam.RateLimitingVerifyBackup/0
[       OK ] RateLimiting/BackupEngineRateLimitingTestWithParam.RateLimitingVerifyBackup/0 (157 ms)
[ RUN      ] RateLimiting/BackupEngineRateLimitingTestWithParam.RateLimitingVerifyBackup/1
[       OK ] RateLimiting/BackupEngineRateLimitingTestWithParam.RateLimitingVerifyBackup/1 (152 ms)
[ RUN      ] RateLimiting/BackupEngineRateLimitingTestWithParam.RateLimitingVerifyBackup/2
[       OK ] RateLimiting/BackupEngineRateLimitingTestWithParam.RateLimitingVerifyBackup/2 (160 ms)
[ RUN      ] RateLimiting/BackupEngineRateLimitingTestWithParam.RateLimitingVerifyBackup/3
[       OK ] RateLimiting/BackupEngineRateLimitingTestWithParam.RateLimitingVerifyBackup/3 (158 ms)
[ RUN      ] RateLimiting/BackupEngineRateLimitingTestWithParam.RateLimitingVerifyBackup/4
[       OK ] RateLimiting/BackupEngineRateLimitingTestWithParam.RateLimitingVerifyBackup/4 (155 ms)
[ RUN      ] RateLimiting/BackupEngineRateLimitingTestWithParam.RateLimitingVerifyBackup/5
[       OK ] RateLimiting/BackupEngineRateLimitingTestWithParam.RateLimitingVerifyBackup/5 (151 ms)
[ RUN      ] RateLimiting/BackupEngineRateLimitingTestWithParam.RateLimitingVerifyBackup/6
[       OK ] RateLimiting/BackupEngineRateLimitingTestWithParam.RateLimitingVerifyBackup/6 (146 ms)
[ RUN      ] RateLimiting/BackupEngineRateLimitingTestWithParam.RateLimitingVerifyBackup/7
[       OK ] RateLimiting/BackupEngineRateLimitingTestWithParam.RateLimitingVerifyBackup/7 (153 ms)
[----------] 8 tests from RateLimiting/BackupEngineRateLimitingTestWithParam (1232 ms total)

mrambacher

My main question/concern is whether part of the code will use a special env over Default and other code will use env over something else. Going through doing something similar on other changes in the past, I found this can lead to weird errors.

mrambacher · 2022-05-10T11:20:16Z

utilities/backup/backup_engine_test.cc

  } else {
    engine_options_->backup_rate_limit = backup_rate_limiter_limit;
  }

+  engine_options_->max_background_operations = is_single_threaded ? 1 : 10;
+
  DestroyDB(dbname_, Options());


Should this Options() refer to special_env?

mrambacher · 2022-05-10T11:21:28Z

utilities/backup/backup_engine_test.cc

+  std::unique_ptr<Env> special_env(
+      new SpecialEnv(Env::Default(), /*time_elapse_only_sleep*/ true));


The code below uses the chroot env. Should the special wrap this env or default?

hx235 · 2022-05-10T17:28:45Z

My main question/concern is whether part of the code will use a special env over Default and other code will use env over something else. Going through doing something similar on other changes in the past, I found this can lead to weird errors.

Good point - I was trying to replace as minimum env as possible but didn't foresee how this inconsistency might lead us to problem (and probably readability issue as reader might be in doubt). Let me think about it more. Thanks!

ajkr

where its clock accepts fake elapse of time during rate-limiting (6582 - 28560 ms/per test)

Is it still waiting for something? Or do these tests do tens of seconds of actual work?

hx235 · 2022-05-12T03:35:03Z

where its clock accepts fake elapse of time during rate-limiting (6582 - 28560 ms/per test)

Is it still waiting for something? Or do these tests do tens of seconds of actual work?

Oh I meant the time spent for each test, not during rate-limiting (updated my description). Most of the time went into backing-up/restoring the db in the test. For that, I shrunk the test db by 10x.

facebook-github-bot · 2022-05-12T03:35:49Z

@hx235 has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator.

mrambacher · 2022-05-12T20:41:31Z

utilities/backup/backup_engine_test.cc

-  FillDB(db_.get(), 0, 100000);
+  TEST_SetDefaultRateLimitersClock(backup_engine_.get(),
+                                   special_env->GetSystemClock(), nullptr);
+  FillDB(db_.get(), 0, 10000);


Was changing the last argument an intended change or accidental>

intended - see updated PR summary for that.

Updated PR title for it too.

mrambacher

LGTM. Thanks!

pdillinger · 2022-05-13T16:21:45Z

Thanks for the improvement!

facebook-github-bot added the CLA Signed label May 10, 2022

hx235 force-pushed the mock_time_backup_test branch 2 times, most recently from d8851e6 to ef47919 Compare May 10, 2022 04:28

hx235 mentioned this pull request May 10, 2022

Support read rate-limiting in SequentialFileReader #9973

Closed

mrambacher reviewed May 10, 2022

View reviewed changes

ajkr reviewed May 11, 2022

View reviewed changes

hx235 force-pushed the mock_time_backup_test branch from ef47919 to 0f49d6c Compare May 12, 2022 03:11

Use fast clock to speed up test && shrink db a bit

98af232

hx235 force-pushed the mock_time_backup_test branch from 0f49d6c to 98af232 Compare May 12, 2022 03:12

hx235 changed the title ~~Use SpecialEnv to speed up some BackupEngineRateLimitingTestWithParam~~ Use SpecialEnv to speed up some slow BackupEngineRateLimitingTestWithParam May 12, 2022

hx235 requested review from mrambacher and pdillinger May 12, 2022 04:05

mrambacher reviewed May 12, 2022

View reviewed changes

hx235 changed the title ~~Use SpecialEnv to speed up some slow BackupEngineRateLimitingTestWithParam~~ Use SpecialEnv + smaller testing db to speed up some slow BackupEngineRateLimitingTestWithParam May 12, 2022

mrambacher approved these changes May 13, 2022

View reviewed changes

facebook-github-bot closed this in e66e6d2 May 16, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Use SpecialEnv + smaller testing db to speed up some slow BackupEngineRateLimitingTestWithParam #9974

Use SpecialEnv + smaller testing db to speed up some slow BackupEngineRateLimitingTestWithParam #9974

hx235 commented May 10, 2022 •

edited

Loading

mrambacher left a comment

mrambacher May 10, 2022

hx235 May 12, 2022

mrambacher May 10, 2022

hx235 May 12, 2022

hx235 commented May 10, 2022

ajkr left a comment

hx235 commented May 12, 2022 •

edited

Loading

facebook-github-bot commented May 12, 2022

mrambacher May 12, 2022

hx235 May 12, 2022 •

edited

Loading

hx235 May 12, 2022

mrambacher left a comment

pdillinger commented May 13, 2022

		std::unique_ptr<Env> special_env(
		new SpecialEnv(Env::Default(), /time_elapse_only_sleep/ true));

Use SpecialEnv + smaller testing db to speed up some slow BackupEngineRateLimitingTestWithParam #9974

Use SpecialEnv + smaller testing db to speed up some slow BackupEngineRateLimitingTestWithParam #9974

Conversation

hx235 commented May 10, 2022 • edited Loading

mrambacher left a comment

Choose a reason for hiding this comment

mrambacher May 10, 2022

Choose a reason for hiding this comment

hx235 May 12, 2022

Choose a reason for hiding this comment

mrambacher May 10, 2022

Choose a reason for hiding this comment

hx235 May 12, 2022

Choose a reason for hiding this comment

hx235 commented May 10, 2022

ajkr left a comment

Choose a reason for hiding this comment

hx235 commented May 12, 2022 • edited Loading

facebook-github-bot commented May 12, 2022

mrambacher May 12, 2022

Choose a reason for hiding this comment

hx235 May 12, 2022 • edited Loading

Choose a reason for hiding this comment

hx235 May 12, 2022

Choose a reason for hiding this comment

mrambacher left a comment

Choose a reason for hiding this comment

pdillinger commented May 13, 2022

hx235 commented May 10, 2022 •

edited

Loading

hx235 commented May 12, 2022 •

edited

Loading

hx235 May 12, 2022 •

edited

Loading