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

CONJ-426 Allow executeBatch to be interrupted #99

Closed

Conversation

kevinconaway
Copy link

@kevinconaway kevinconaway commented Feb 7, 2017

@rusher this PR attempts to address the issue identified in https://jira.mariadb.org/browse/CONJ-426

I believe that the cause of this that AbstractMultiSend#executeBatchStandard swallows the InterruptedException and does not reset the threads interrupted status. Thus the caller thread has no idea whether its been interrupted or not.

The executeBatch call returns early due to the interrupt but the query is actually running the background. Thus there is a race condition when querying the state of the cmdInformation in `MariaDbServerPreparedStatement#executeBatch. This manifests as the NPE that I linked in the ticket.

My fix in this PR addresses all of this by modifying AsyncMultiSend to properly catch the InterruptedException, reset the thread interrupt state and rethrow the exception as a QueryException. I also modified this class to cancel the ongoing AsyncMultiRead task which is also now interruptible.

I've also modified the data structures in CmdInformationMultiple to be thread safe since there is still a tiny chance that the AsyncMultiRead task will attempt to write a result while the caller thread in MariaDbServerPreparedStatement will attempt to query them.

…ultiple data structures for concurrent access
@rusher
Copy link
Collaborator

rusher commented Feb 7, 2017

Hi @kevinconaway,

Great addition,

Similar to other open source projects, the MariaDB Foundation needs to have shared ownership of all code that is included in the MariaDB distribution. The easiest way to achieve this is by submitting your code under the BSD-new license. (The other alternative is to sign the code contribution agreement which can be found here: https://mariadb.com/kb/en/mariadb/mca/)

Please indicate in a comment below that you are contributing your new code of the whole pull request, including one or several files that are either new files or modified ones, under the BSD-new license or that you have filled out the contribution agreement and sent it.

Thanks,
diego

…rewritten and normal batch statements to be interrupted. Increase batch size for test because rewritten batch statements run faster.
@kevinconaway
Copy link
Author

Please indicate in a comment below that you are contributing your new code of the whole pull request, including one or several files that are either new files or modified ones, under the BSD-new license

Please consider all commits and comments on this work submitted under the BSD-new license.

The other alternative is to sign the code contribution agreement which can be found here: https://mariadb.com/kb/en/mariadb/mca/

Do I need a username on on Launchpad / askmonty.org in order to sign this?

@kevinconaway
Copy link
Author

I've also submitted a copy of the MCA

@kevinconaway
Copy link
Author

I've modified my test case to only run when useBatchMultiSend=true. The normal protocol uses blocking socket IO which is not interruptible.

@kevinconaway
Copy link
Author

The build failures linked here https://travis-ci.org/MariaDB/mariadb-connector-j/builds/199340719 seem to be due to ssl / connectivity issues when pulling maven dependencies, not my changes.

@rusher
Copy link
Collaborator

rusher commented Feb 8, 2017

@kevinconaway, yes, maven/connection on travis did have some random issue yesterday.
This PR is merged in branch FIX1.5.8 with some changes to always have connection in a correct state.

@rusher
Copy link
Collaborator

rusher commented Feb 20, 2017

closing, since merge in 1.5.8 (722f717)

@rusher rusher closed this Feb 20, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants