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

Batch PreparedStatements in SQL status updater bolt #610

Closed
jnioche opened this Issue Sep 24, 2018 · 6 comments

Comments

Projects
None yet
2 participants
@jnioche
Member

jnioche commented Sep 24, 2018

Each document is currently updated in an individual call to SQL, which can be a bottleneck. It would be more efficient to batch the preparedstatements.

@jnioche

This comment has been minimized.

Member

jnioche commented Sep 25, 2018

@jnioche

This comment has been minimized.

@cruftex

This comment has been minimized.

Contributor

cruftex commented Sep 26, 2018

Hi Julien,

unfortunately there are no automated tests for the SQL persistence component. That would make working on code improvements so much easier.

In our applications we use the H2 database engine with in memory mode for the tests and PostgreSQL in production. That works pretty fine. For the tests there is no need to setup a database then, which means it can run on every CI server.

Another thing: There are some MySQL specific functions used. I am not 100% sure, but it seems everything needed is pretty basic, so it should be possible to make everything database agnostic and just use plain SQL.

So I'd recommend to get rid of MySQL specifics and have some tests first. What do you think?

@cruftex

This comment has been minimized.

Contributor

cruftex commented Sep 26, 2018

Another thing: Some SQL databases have JSON support. It would be a good idea to change the metadata serialization format to JSON.

@jnioche

This comment has been minimized.

Member

jnioche commented Sep 26, 2018

Hi Jens, thanks for your comments. I like the idea of having tests + be database neutral.
Am working on a first version of the batching, will send a PR shortly. Would be great if you could review it, we can then refine it further.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment