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 "housekeeping" tasks for db opened with openAgain() #2225

Draft
wants to merge 3 commits into
base: release/3.1
Choose a base branch
from

Conversation

jianminzhao
Copy link
Contributor

No description provided.

jianminzhao and others added 2 commits February 7, 2025 11:07
Adds a flag `C4DB_NoHousekeeping` that suppresses database
"housekeeping" tasks, and makes C4Database::openAgain() set this
flag in the database it opens.

Housekeeping tasks like expiring documents and SQLite vacuuming
only need to be done by one database connection (C4Database), but we
do them in all connections. This means CBL wastes some time and also
opens twice as many SQLite connections as it needs to (that
_backgroundDB that DatabaseImpl opens.)

It also enables a nasty deadlock condition when the replicator closes
its database(s) -- the close call happens on an actor thread, and
ends up calling Housekeeper::stop, which sends an Actor message to
the Housekeeper and blocks until it's processed. But if all actor
threads are doing this, there are no threads left to actually
run the Housekeeper. [CBSE-19205]
@jianminzhao jianminzhao marked this pull request as draft February 7, 2025 19:44
@jianminzhao jianminzhao requested review from snej and pasin February 7, 2025 19:44
kC4DB_ReadOnly = 0x02, ///< Open file read-only
kC4DB_AutoCompact = 0x04, ///< Enable auto-compaction [UNIMPLEMENTED]
kC4DB_VersionVectors = 0x08, ///< Upgrade DB to version vectors instead of rev trees
kC4DB_MmapDisabled = 0x10, ///< Disable MMAP in SQLite.
Copy link
Collaborator

Choose a reason for hiding this comment

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

kC4DB_MmapDisabled shouldn't be ported.

@pasin
Copy link
Collaborator

pasin commented Feb 7, 2025

There is a conflicting file as well.

Copy link
Collaborator

@snej snej left a comment

Choose a reason for hiding this comment

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

I don't understand why you had to make additional changes due to compiler warnings. Didn't the branch build successfully before this?

Comment on lines 153 to 155
options.diskSyncFull = (_config.flags & kC4DB_DiskSyncFull) != 0;
options.noHousekeeping = (_config.flags & kC4DB_NoHousekeeping) != 0;
options.useDocumentKeys = true;
Copy link
Collaborator

Choose a reason for hiding this comment

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

Why switch the order of useDocumentKeys and diskSyncFull?

@pasin pasin requested review from bmeike and removed request for bmeike February 7, 2025 20:05
@jianminzhao
Copy link
Contributor Author

"I don't understand why you had to make additional changes due to compiler warnings. Didn't the branch build successfully before this?" -- because in won't pass the compiler with my current Xcode version&setting.

@jianminzhao jianminzhao requested review from snej and pasin February 7, 2025 20:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants