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

WIP: Use SQLite instead of PostgreSQL #19852

Closed

Conversation

cdelafuente-r7
Copy link
Contributor

This is a first try to migrate to SQLite. It works locally, but, let see if CI passes 🤞

@adfoster-r7
Copy link
Contributor

If you pop this value to false it should allow us to run all the test slices and gather the results together:

@cdelafuente-r7 cdelafuente-r7 marked this pull request as draft February 4, 2025 12:42
@cdelafuente-r7
Copy link
Contributor Author

CI is failing due to concurrency issues, which is a known problem with SQLite since it only accepts one connection to acquire a write lock on the database. This causes a SQLite3::BusyException: database is locked exception to be raised.

Rails version 8 had added many improvements and now ships with SQLite as the default database, considered Ready for Production. Since we still use rails version 7.0.x, I believe upgrading it to version 8 would probably solve these issues.

Another solution is to use this gem, which enhance ActiveRecord's SQLite3 adapter (see this article for details), but it requires rails version 7.1, which might be possible soon (see this PR)

@adfoster-r7
Copy link
Contributor

The 7.1 upgrade is actually blocked on database connection issues, so we might need to resolve them first. It might be possible to have a test branch with the current 7.1 effort and a SQLite spike to confirm if that's enough for us or not, but I'm happy to park this PR until we're officially on at least 7.1 if that's your recommendation 👍

@cdelafuente-r7
Copy link
Contributor Author

Thank you! I just quickly tested on top of the branch with the rails 7.1 upgrade and installed the activerecord-enhancedsqlite3-adapter gem. Unfortunately, we still have issues with SQLite. So, the problem might be deeper than I thought and more investigation will be needed.

Concurrency issues like these are usually due to unclosed transactions with write operations. We will probably need to review how we handle transactions in a multi-threaded environment to get rid of these concurrency errors.

@adfoster-r7
Copy link
Contributor

That's perfect; thanks for taking a look! I'd be good to temporarily attic this PR for now as needing to be revisited after we've upgraded to a newer version of Rails 💯

Let me know if you'd be for or against popping the attic label on the PR for now 🎉

@cdelafuente-r7
Copy link
Contributor Author

Yes, I agree. Let's attic this PR for now.

@adfoster-r7 adfoster-r7 added the attic Older submissions that we still want to work on again label Feb 4, 2025
Copy link

github-actions bot commented Feb 4, 2025

Thanks for your contribution to Metasploit Framework! We've looked at this pull request, and we agree that it seems like a good addition to Metasploit, but it looks like it is not quite ready to land. We've labeled it attic and closed it for now.

What does this generally mean? It could be one or more of several things:

  • It doesn't look like there has been any activity on this pull request in a while
  • We may not have the proper access or equipment to test this pull request, or the contributor doesn't have time to work on it right now.
  • Sometimes the implementation isn't quite right and a different approach is necessary.

We would love to land this pull request when it's ready. If you have a chance to address all comments, we would be happy to reopen and discuss how to merge this!

@github-actions github-actions bot closed this Feb 4, 2025
@adfoster-r7
Copy link
Contributor

Thanks again! Hoping to revive this soon as it would be awesome to have working for our users 🤞

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
attic Older submissions that we still want to work on again
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants