From 0447a04bc71df897015722bc57a5f499cdcfab2a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Adrien=20Verg=C3=A9?= Date: Mon, 28 Jan 2019 10:43:43 +0100 Subject: [PATCH] Compaction: Add snooze_period_ms for finer tuning We are experiencing an issue similar to #1579, partly because we have dozens of thousands of databases. On our servers: - `snooze_period = 0` leads to a significant CPU load, - `snooze_period = 1` is too long for compaction to finish within 20 hours (after 20 hours, compaction is stopped to allow other CPU-intensive processes to run, and when compaction restarts, it does not pick up where it left 4 hours earlier -- by the way proposal at #1775 would be really great to fix that!) This commit introduces a new option `snooze_period_ms` (measured in milliseconds), and deprecates `snooze_period` while still supporting it for obvious legacy reasons. --- rel/overlay/etc/default.ini | 7 +- src/couch/src/couch_compaction_daemon.erl | 71 +++++++++++++++++-- .../test/couchdb_compaction_daemon_tests.erl | 2 +- 3 files changed, 72 insertions(+), 8 deletions(-) diff --git a/rel/overlay/etc/default.ini b/rel/overlay/etc/default.ini index b9d51af7194..24428f3e51b 100644 --- a/rel/overlay/etc/default.ini +++ b/rel/overlay/etc/default.ini @@ -434,9 +434,10 @@ min_file_size = 131072 ; With lots of databases and/or with lots of design docs in one or more ; databases, the compaction_daemon can create significant CPU load when ; checking whether databases and view indexes need compacting. The -; snooze_period setting ensures a smoother CPU load. Defaults to -; 3 seconds wait. -; snooze_period = 3 +; snooze_period_ms setting ensures a smoother CPU load. Defaults to +; 3000 milliseconds wait. Note that this option was formerly called +; snooze_period, measured in seconds (it is currently still supported). +; snooze_period_ms = 3000 [compactions] ; List of compaction rules for the compaction daemon. diff --git a/src/couch/src/couch_compaction_daemon.erl b/src/couch/src/couch_compaction_daemon.erl index 115a9a89741..2a46c3f26fe 100644 --- a/src/couch/src/couch_compaction_daemon.erl +++ b/src/couch/src/couch_compaction_daemon.erl @@ -125,8 +125,16 @@ handle_config_terminate(_, stop, _) -> handle_config_terminate(_Server, _Reason, _State) -> erlang:send_after(?RELISTEN_DELAY, whereis(?MODULE), restart_config_listener). +get_snooze_period() -> + % The snooze_period_ms option should be used, but snooze_period is supported + % for legacy reasons. + Default = config:get_integer("compaction_daemon", "snooze_period", 3), + case config:get_integer("compaction_daemon", "snooze_period_ms", -1) of + -1 -> Default * 1000; + SnoozePeriod -> SnoozePeriod + end. + compact_loop(Parent) -> - SnoozePeriod = config:get_integer("compaction_daemon", "snooze_period", 3), {ok, _} = couch_server:all_databases( fun(DbName, Acc) -> case ets:info(?CONFIG_ETS, size) =:= 0 of @@ -140,7 +148,7 @@ compact_loop(Parent) -> case check_period(Config) of true -> maybe_compact_db(Parent, DbName, Config), - ok = timer:sleep(SnoozePeriod * 1000); + ok = timer:sleep(get_snooze_period()); false -> ok end @@ -231,8 +239,7 @@ maybe_compact_views(DbName, [DDocName | Rest], Config) -> timeout -> ok end, - SnoozePeriod = config:get_integer("compaction_daemon", "snooze_period", 3), - ok = timer:sleep(SnoozePeriod * 1000); + ok = timer:sleep(get_snooze_period()); false -> ok end. @@ -597,4 +604,60 @@ abs_path2_test() -> ?assertEqual({ok, "/a/b/"}, abs_path2("/a/b")), ok. +get_snooze_period_test_() -> + { + foreach, + fun() -> + meck:new(config, [passthrough]) + end, + fun(_) -> + meck:unload() + end, + [ + {"should return default value without config attributes", + fun should_default_without_config/0}, + {"should respect old config attribute", + fun should_respect_old_config/0}, + {"should respect old config set to zero", + fun should_respect_old_config_zero/0}, + {"should respect new config attribute", + fun should_respect_new_config/0}, + {"should respect new config set to zero", + fun should_respect_new_config_zero/0} + ] + }. + +should_default_without_config() -> + ?assertEqual(3000, get_snooze_period()). + +should_respect_old_config() -> + meck:expect(config, get_integer, fun + ("compaction_daemon", "snooze_period", _) -> 1; + (_, _, Default) -> Default + end), + ?assertEqual(1000, get_snooze_period()). + +should_respect_old_config_zero() -> + meck:expect(config, get_integer, fun + ("compaction_daemon", "snooze_period", _) -> 0; + (_, _, Default) -> Default + end), + ?assertEqual(0, get_snooze_period()). + +should_respect_new_config() -> + meck:expect(config, get_integer, fun + ("compaction_daemon", "snooze_period", _) -> 1; + ("compaction_daemon", "snooze_period_ms", _) -> 300; + (_, _, Default) -> Default + end), + ?assertEqual(300, get_snooze_period()). + +should_respect_new_config_zero() -> + meck:expect(config, get_integer, fun + ("compaction_daemon", "snooze_period", _) -> 1; + ("compaction_daemon", "snooze_period_ms", _) -> 0; + (_, _, Default) -> Default + end), + ?assertEqual(0, get_snooze_period()). + -endif. diff --git a/src/couch/test/couchdb_compaction_daemon_tests.erl b/src/couch/test/couchdb_compaction_daemon_tests.erl index c10ddee1253..0ef2a406414 100644 --- a/src/couch/test/couchdb_compaction_daemon_tests.erl +++ b/src/couch/test/couchdb_compaction_daemon_tests.erl @@ -24,7 +24,7 @@ start() -> Ctx = test_util:start_couch(), ok = config:set("compaction_daemon", "check_interval", "3", false), - ok = config:set("compaction_daemon", "snooze_period", "0", false), + ok = config:set("compaction_daemon", "snooze_period_ms", "0", false), ok = config:set("compaction_daemon", "min_file_size", "100000", false), ok = config:delete("compactions", "_default", false), ok = meck:new(?MODS_TO_MOCK, [passthrough]),