/diesel

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

Ensure batch inserts are done in a transaction on SQLite #1177

Closed
sgrif opened this Issue Sep 19, 2017 · 0 comments

Comments

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
1 participant
@sgrif
@sgrif
Member

sgrif commented Sep 19, 2017

There's a significant performance difference between doing a batch insert in a transaction vs out of a transaction on SQLite, since if we're not in a transaction it'll write to disk between every query. The implementation will need to be a little bit more complex than just conn.transaction(|| what we have now), since savepoints have a non-zero cost so we should only start a transaction if we aren't already inside of one.

@sgrif sgrif referenced this issue Sep 19, 2017

Closed

feat: Support batch insert for sqlite #1160

sgrif added a commit that referenced this issue Sep 21, 2017

@sgrif
Ensure batch inserts on SQLite run in a transaction 
While it's safe for us to do the inserts one query at a time, due to the
lack of round-trip time on SQLite, there's still a significant
performance difference if done outside of a transaction. If we're not in
a transaction, SQLite will perform table locking, and file IO for each
query individually.

This also means that batch insert on SQLite will have the same semantics
on failure as the other backends.

While I previously thought that we did not want a savepoint here if we
were already in a transaction, it's actually important that we create
one in order to match the semantics of other backends on failure.

Fixes #1177.
Loading status checks…
48e21f6

@sgrif sgrif referenced this issue Sep 21, 2017

Merged

Ensure batch inserts on SQLite run in a transaction #1183

sgrif added a commit that referenced this issue Sep 21, 2017

@sgrif
Ensure batch inserts on SQLite run in a transaction 
While it's safe for us to do the inserts one query at a time, due to the
lack of round-trip time on SQLite, there's still a significant
performance difference if done outside of a transaction. If we're not in
a transaction, SQLite will perform table locking, and file IO for each
query individually.

This also means that batch insert on SQLite will have the same semantics
on failure as the other backends.

While I previously thought that we did not want a savepoint here if we
were already in a transaction, it's actually important that we create
one in order to match the semantics of other backends on failure.

Fixes #1177.
Loading status checks…
f3607d3

sgrif added a commit that referenced this issue Sep 21, 2017

@sgrif
Ensure batch inserts on SQLite run in a transaction 
While it's safe for us to do the inserts one query at a time, due to the
lack of round-trip time on SQLite, there's still a significant
performance difference if done outside of a transaction. If we're not in
a transaction, SQLite will perform table locking, and file IO for each
query individually.

This also means that batch insert on SQLite will have the same semantics
on failure as the other backends.

While I previously thought that we did not want a savepoint here if we
were already in a transaction, it's actually important that we create
one in order to match the semantics of other backends on failure.

Fixes #1177.
Loading status checks…
88b7412

sgrif added a commit that referenced this issue Sep 21, 2017

@sgrif
Ensure batch inserts on SQLite run in a transaction 
While it's safe for us to do the inserts one query at a time, due to the
lack of round-trip time on SQLite, there's still a significant
performance difference if done outside of a transaction. If we're not in
a transaction, SQLite will perform table locking, and file IO for each
query individually.

This also means that batch insert on SQLite will have the same semantics
on failure as the other backends.

While I previously thought that we did not want a savepoint here if we
were already in a transaction, it's actually important that we create
one in order to match the semantics of other backends on failure.

Fixes #1177.
Loading status checks…
b0cf840

sgrif added a commit that referenced this issue Sep 21, 2017

@sgrif
Ensure batch inserts on SQLite run in a transaction 
While it's safe for us to do the inserts one query at a time, due to the
lack of round-trip time on SQLite, there's still a significant
performance difference if done outside of a transaction. If we're not in
a transaction, SQLite will perform table locking, and file IO for each
query individually.

This also means that batch insert on SQLite will have the same semantics
on failure as the other backends.

While I previously thought that we did not want a savepoint here if we
were already in a transaction, it's actually important that we create
one in order to match the semantics of other backends on failure.

Fixes #1177.
Loading status checks…
59f59e3

@killercup killercup closed this in #1183 Sep 22, 2017

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