worldveil / dejavu

Audio fingerprinting and recognition in Python
MIT License
6.41k stars 1.43k forks source link

Infinite loop if MySQL server connection timed out. #7

Closed Wessie closed 10 years ago

Wessie commented 10 years ago

When the MySQL server hangs up on the connection and dejavu tries to use it, it will print out Error in query(), 2006: MySQL server has gone away until stopped by the user.

Traceback after using an interrupt:

Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "dejavu/recognize.py", line 38, in read
  File "dejavu/fingerprint.py", line 62, in match

  File "dejavu/database.py", line 320, in return_matches
  File "dejavu/database.py", line 288, in query

The database connection should most likely be dropped after usage, and (re)created when needed. Instead of connecting at initialization and holding onto forever.

worldveil commented 10 years ago

Excellent point. At what point should it be dropped? Perhaps a better solution is to have a try/catch and if a 2006 comes up to recreate the connection

Wessie commented 10 years ago

I have a context manager for this somewhere, it caches connections and knows when to reconnect. It should also be able to clean-up a lot of the try: except: clauses that are around a lot of the MySQL interactions.

I'll be working on a pull request if that is okay.

worldveil commented 10 years ago

Sounds great.

Wessie commented 10 years ago

After some more playing around, a friend and myself ended up editing dejavu quite heavily over the past 2 days. You can look at the changes in my fork (here).

I would love to merge them back into the main repository but the changes are quite big (basically a rewrite exempting the algorithm). It's up to you to decide if you want to accept the changes. Feel free to ask any questions as well!

worldveil commented 10 years ago

Actually the changes look great. A lot of things I haven't had time to do. Perhaps you also want to rewrite the go.py to show off some of the new things - different recognizers, new syntax etc. Then make a pull request and I can take a closer look at all changes.

The biggest thing I don't see (but may have missed) was making sure the 'autocommit' was turned off by default, otherwise inserting hashes into mysql is much, much slower.

Also is the Queue in the cursor module to allow for processes to share?

Wessie commented 10 years ago

I've updated go.py to contain some examples of usage now.

The Queue is for sharing and caching of connections across threads, each process will have its own Queue.

I would like to emphasize that the fork isn't perfect, and it still has some rough edges. Especially error handling has to be looked at (and documented).

worldveil commented 10 years ago

Looks great. Only thing is I'm a bit worried we'll blow away pgurdi's latest stuff which only fingerprints first n seconds of files and lets you conditionally keep/delete converted mp3 to wav files. But if you PR I can add that to your fork, or if you are inspired you can.

Re: error handling, any particular try/catches you are referring to?