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

Draft PostgreSQL support #3232

Draft
wants to merge 17 commits into
base: develop
Choose a base branch
from
Draft

Draft PostgreSQL support #3232

wants to merge 17 commits into from

Conversation

kainz
Copy link
Contributor

@kainz kainz commented Nov 4, 2019

So I've had a branch with PostgreSQL support for headphones sitting around for a couple years now, thought I'd update it.

Why did I do such a thing?

  • I wanted to see if pypy sped up some of the api and searches
  • I got frustrated seeing the query timeouts on locking and other sqlite oddities
  • I wanted to see how different some of the index search speed was in PostgreSQL.

I would not call this a clean implementation, it has a few glaring issues:

  • types are still all textual
  • we have to avoid using COLLATE NOCASE, but thats not really a problem anyway, as COLLATE NOCASE does not support Unicode/UTF-8.
  • A few of the queries don't translate cleanly between SQLite and PostgreSQL. I hope I've denoted those, but this branch currently breaks sqlite support as a side effect.
  • Still no ORM or real migration support

I'd like to take some of the bits I've identified here, and maybe combine them to add SQLAlchemy support and get something close to real cross-db compatibility. Is there any interest upstream in such a feature?

At least in my local use case, this work has given me a definitely better experience running headphones, as I have no (sqlite) locking delays between the various threads headphones runs. Some queries/searches can also be faster, but I haven't dived really deep into this for awhile.

This branch can also run successfully on pypy, but I have not benchmarked it in awhile to see how different the speed would be.

Currently, I've been testing this branch on pypy 2.7 (pypy7.0.0), and PostgreSQL 9.5. I'm of a mind to upgrade to 9.6+ though, as 9.6 adds alter table add column if not exists syntax.

Thoughts?

kainz added 17 commits July 14, 2016 06:29
Implement some core api bits around postgresql support.  For this to
work, binding formats will have to be changed throughout the rest of the
source, hoping this will work for mitigating that.  Future commits will
do the binding parameter changes, and alter some of the fetches to be
consistent with psql's and sqlite's expectations.
Convert all the sql uses in the python codebase to use syntax compatible
with both cursors, and to do some more explicit transaction management.

The transaction management is messy as hell, but at least its a start,
and not necessarily as lock/performance intensive as full-on autocommit.
Also fix pythony sqlite-specific bits here.
COLLATE NOCASE is not great to use in many systems:
 * it only exists for SQLite
 * Furthermore, it only works for ASCII.
psql rollback semantics are different than sqlite's, so we need to
rollback after test 'failures' in the DB migration phase.

use a flag (a table/timestamp in this case) to detect when we've done
the artists lowercase transform as well, so we don't get bit if the
first artistname collation happens to be lowercased. (unlikely on most
libraries, but definitely possible).
my in-db optimization for 'havetracks' wasn't using the query result
correctly
@monik3r
Copy link

monik3r commented Feb 11, 2020

Hi,

I've been looking into this branch for testing, I have a pretty large headphones instance and it tends to grind a bit. I've attempted to use this dockerfile: https://gist.github.com/monik3r/a7f68676582c5e4837702ddfff9a79c0 , but the server won't not reset the connection on each attempt.

Do you have any details on how to set this up? Should I be setting up a postgres db with some form of creds?

Thanks, it would be really neat to try out your code (and benchmark it)!

Edit: checked the head of the logs and found an error that was being returned from the app, I'll keep looking into it, here's the log: https://gist.github.com/monik3r/0ec924638bfe8b4311c7ccaf379f0a1c

Edit 2: Managed to get everything working, had to hardcode in some server values for now to get the webserver started, as well as change localhost to 0.0.0.0 - thee were likely caused by me during deployment. I'll let it run for a few hours and see how it performs compared to the current maser headphones

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