Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Disable compaction daemon on eunit run couch startups #654

Merged
merged 1 commit into from
Jul 8, 2017

Conversation

wohali
Copy link
Member

@wohali wohali commented Jul 8, 2017

Commit 21f9544 enabled the compaction daemon by default. And commit
3afe3ad disabled the compaction daemon at the start of the compaction
daemon tests. Unfortunately, the compaction daemon remains active during
all the other EUnit tests. This has resulted in some strange behaviour,
especially filling the LRU up and all_dbs_active errors during tests
that shouludn't be hitting it otherwise (like
couchdb_views_tests:couchdb_1283 which monkeys with max_dbs_open ).

I attempted to override the [compactions] _default line in the file
rel/files/eunit.ini but specifying _default= or _default=[] did not
provide the desired behaviour. (This in and of itself is worrying; how would
someone disable the compaction daemon in local.ini now?)

This change disables the _default config as part of
test_util:start_couch after startup. This means there is a very brief
period during which the daemon is running, but in empirical testing the
only thing I've seen it manage to do is compact _dbs before being
disabled.

Tested with make soak-eunit for 3 hours; this seems to eliminate the
all_dbs_active error we've been seeing in CI runs since the compaction
daemon was enabled by default.

Now, I realize that this PR is at odds with making sure that all of the
usual things we do in CouchDB work correctly while the compaction daemon
is running in the background, so if you feel strongly about this, don't
hesitate to -1 it.

Noting here that the alternative is that we can try and fix all of the
suddenly failing tests - but with all of the DBs that EUnit creates and
deletes rapidly, the daemon will be working overtime to try and compact
them all. And many EUnit tests don't delete their databases after
they're done (which itself is probably another bug). The LRU is
adversely affected as well, as we see in
couchdb_views_tests:couchdb_1283, but
couch_db_tests:should_create_multiple_dbs would also be affected.

Other test failures from enabling the compaction daemon:

Closes #652

Commit 21f9544 enabled the compaction daemon by default. And commit
3afe3ad disabled the compaction daemon at the start of the compaction
daemon tests. Unfortunately, the compaction daemon remains active
during all the other EUnit tests.

I attempted to override the [compactions] _default line in the file
rel/files/eunit.ini but specifying `_default=` or `_default=[]` did not
provide the desired behaviour.

This change disables the _default config as part of
`test_util:start_couch` after startup. This means there is a very
brief period during which the daemon is running, but in empirical
testing the only thing I've seen it manage to do is compact `_dbs`
before being disabled.

Tested with `make soak-eunit` for 3 hours; this seems to eliminate
the `all_dbs_active` error we've been seeing in CI runs since the
compaction daemon was enabled by default.
@wohali wohali requested review from janl and davisp July 8, 2017 00:06
@wohali wohali added this to the 2.1.0 milestone Jul 8, 2017
Copy link
Member

@janl janl left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

+1

@wohali wohali merged commit da7aa54 into master Jul 8, 2017
@wohali wohali deleted the 652-disable-compaction-daemon branch July 8, 2017 06:19
@davisp
Copy link
Member

davisp commented Jul 9, 2017

+1

I don't think we should worry about disabling the daemon for tests. Adding randomness to the tests when we're making assertions about global state isn't gonna work so hot when we've got something doing things in the background without coordination. If we're worried about testing things with the daemon running we should just write new tests for it rather than screw with all the existing tests.

nickva pushed a commit to nickva/couchdb that referenced this pull request Sep 7, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Compaction daemon is interfering with eunit tests (all_dbs_active errors)
3 participants