Skip to content

Catalyst fixes#52

Merged
atdrendel merged 2 commits intomainfrom
catalyst-fixes
Jan 30, 2024
Merged

Catalyst fixes#52
atdrendel merged 2 commits intomainfrom
catalyst-fixes

Conversation

@atdrendel
Copy link
Copy Markdown
Contributor

@atdrendel atdrendel merged commit ed19230 into main Jan 30, 2024
@atdrendel atdrendel deleted the catalyst-fixes branch January 30, 2024 22:27
config.defaultTransactionKind = .immediate
// NOTE: GRDB recommends `defaultTransactionKind` be set
// to `.immediate` in order to prevent `SQLITE_BUSY`
// errors. Using `.immediate` appears to disable
Copy link
Copy Markdown

@groue groue Jan 31, 2024

Choose a reason for hiding this comment

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

Hello @atdrendel, thanks for trusting the GRDB doc :-) May I ask how you have discovered that immediate transactions disable automatic vacuuming? I never use PRAGMA auto_vacuum, out of sheer inexperience. It looks like immediate transactions can have a big cost in terms of disk usage for some apps?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

@groue My tests caught the problem with automatic vacuuming. When .immediate is set, the auto_vacuum pragma seems to have to effect.

I just released a nightly with this change last night. So, I can't really comment on its performance yet. I'll keep an eye on it, though.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Also, my vacuum tests all failed when I enabled .immediate. I haven't looked into that yet, either, but I guess the disk size issue you mentioned could be related to that.

Copy link
Copy Markdown

@groue groue Jan 31, 2024

Choose a reason for hiding this comment

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

A search in Google and the SQLite forums does not make clear that IMMEDIATE transactions are linked to auto-vacuum.

But this post is probably related: https://sqlite.org/forum/forumpost/4cc6473ceb9038cf618282e40f235c4f35f0c08832de0bcb276f84bff6745688

The auto-vacuum only happens on a write transaction. For a read transaction, nothing has changed, and so there is nothing to auto-vacuum. Hence the write lock is already held whenever an auto-vacuum occurs.

In WAL mode, the database changes necessary to move freelist pages to the end of the database and then truncate the database are written into the WAL file. But those changes are not actually applied (and the database does not actually shrink) until the next checkpoint operation is run to move the changes in the WAL file back into the database.

Indeed I have already seen that VACUUM (auto or not) does not reduce the size of a WAL database, because… the VACUUM is registered in the WAL (I'm not discussing if it is the expected behavior or not 😅). Maybe this is what your tests have detected? Something related to the WAL mode, more than something related to immediate transactions?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

@groue I found some time to look into this issue today. The vacuuming issue was not directly caused by the IMMEDIATE transaction change. The reason for the change was my library was incorrectly using writer.write in cases when I should have used writer.writeWithoutTransaction. I addressed that problem in #53 and now all of the tests pass correctly.

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Thank you for your investigations 🙏

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.

2 participants