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

Implement auto-reconnection to SQL database #11

Closed
wants to merge 1 commit into from

Conversation

dermoth
Copy link
Member

@dermoth dermoth commented Feb 26, 2013

Again a fairly untested commit on non-MySQL databases, but fixes the annoying issue where the server needs to be restarted whenever the connection to MySQL is lost. More testing on other SQL's would be great. Here's what I suggest for testing:

  1. Start Abe, open a page on it (ex block or something like it, needs to retrieve data from database).
  2. Restart the sql server, test reconnection works as expected by reloading the page. Reconnection should happen immediately, you should get no error on first reload.
  3. Shutdown the sql server, and reload the page, you should get an error
  4. Restart the sql server, test reconnection still as expected by reloading the page (like # 2)

Commit message below...

Tested against MySQL, but should work on other databases as well.

OperationalError seems to be the standard errors when initially loosing
the connection or trying a rollback() on a dead con. However if the
reconnect fails we end up with a stale cursor, and then MySQL returns a
ProgrammingError. InternalError should be used then the cursor is not
valid anymore which warrants a reconnect regardless.

Tested against MySQL, but should work on other databases as well.

OperationalError seems to be the standard errors when initially loosing
the connection or trying a rollback() on a dead con. However if the
reconnect fails we end up with a stale cursor, and then MySQL returns a
ProgrammingError. InternalError should be used then the cursor is not
valid anymore which warrants a reconnect regardless.
@dermoth
Copy link
Member Author

dermoth commented Feb 26, 2013

Oh one thing I realize I haven't looked at there is the possibility we reconnect in the middle of a transaction. I'm not sure how big this is an issue, and I'm wondering how we should handle this. Maybe create an ImplicitRollback exception and raise it when we do a reconnect, then just "pass" on it wherever we don't rely on transactions...

And worth mentioning, I realize now I did all my tests with abe.store.catch_up() commented out as i'm running it in a separate cronjob...

@jtobey
Copy link
Member

jtobey commented Mar 15, 2013

Thanks and sorry to take so long to comment. I think this will work, provided that all drivers follow the DB-API by defining module.OperationalError, etc. However, even though the chance seems remote, I do worry about a reconnect during a transaction. Since v0.1, I've always erred on the side of caution with respect to corrupting the database and requiring a full reload.

Short story: Abe should (but does not) remember when it is in a transaction and know when it is safe to reconnect. Could it be as simple as clearing a flag on connect/commit/rollback and setting it on other SQL?

While we're at it, Abe should choose an isolation level, test whether it works, and either explicitly start transactions or retry deadlocks, depending on the level chosen, but this is getting off topic.

Edit: As for commenting out catch_up(), I just pass "--datadir=[]" these days. This (separate server and loader processes) is probably worth a new README.

@Hartland
Copy link

I have had an issue when using this commit (patching the latest ABE version) and new databases. The commit works great when the database is already established, but when starting a fresh database, DDL implicit commit errors arise. If I build the database with an unpatched version of DataStore.py and then patch/replace DataStore.py with the commit, everything works.

Example:

robert@vm:~/fc/Abe$ python abe.py --config abe-my.conf --commit-bytes 100000 --no-serve
Reconnecting to database.
Reconnecting to database.
Reconnecting to database.
Reconnecting to database.
Traceback (most recent call last):
File "abe.py", line 2192, in
sys.exit(main(sys.argv[1:]))
File "abe.py", line 2186, in main
store = make_store(args)
File "abe.py", line 140, in make_store
store = DataStore.new(args)
File "/home/robert/fc/Abe/DataStore.py", line 3216, in new
return DataStore(args)
File "/home/robert/fc/Abe/DataStore.py", line 156, in init
store.initialize()
File "/home/robert/fc/Abe/DataStore.py", line 971, in initialize
store.configure()
File "/home/robert/fc/Abe/DataStore.py", line 1280, in configure
store.configure_ddl_implicit_commit()
File "/home/robert/fc/Abe/DataStore.py", line 1359, in configure_ddl_implicit_commit
raise Exception("Can not test for DDL implicit commit.")
Exception: Can not test for DDL implicit commit.

@jtobey
Copy link
Member

jtobey commented Apr 22, 2013

Thanks for the bug report. Please try the latest commit (6e5f9ee) unpatched. This tries to reconnect after failed DML statements at the start of a transaction. I would think that is when timeouts normally happen. Auto-reconnection at other times would seem unhelpful (e.g., during schema initialization) or potentially dangerous (in mid-transaction).

@jtobey
Copy link
Member

jtobey commented May 10, 2013

Implemented for "safe" cases. Please open an issue if the problem happens again.

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.

None yet

3 participants