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

Support for traditional and consecutive behavior for innodb_autoinc_lock_mode #7634

Closed
arvidfm opened this issue Mar 21, 2024 · 12 comments · Fixed by #7704
Closed

Support for traditional and consecutive behavior for innodb_autoinc_lock_mode #7634

arvidfm opened this issue Mar 21, 2024 · 12 comments · Fixed by #7704
Assignees
Labels
customer issue enhancement New feature or request sql Issue with SQL

Comments

@arvidfm
Copy link

arvidfm commented Mar 21, 2024

For MySQL we're currently checking innodb_autoinc_lock_mode because our system relies on it being set to traditional (0) or consecutive (1) for bulk inserts to behave as expected, but it seems Dolt doesn't support it:

> SELECT @@GLOBAL.innodb_autoinc_lock_mode;
ERROR 1105 (HY000): Unknown system variable '@@global.innodb_autoinc_lock_mode'

I also couldn't find any documentation on how Dolt handles autoinc IDs by default in regards to locking and interleaving, besides the risk of conflicts across distinct clones (which is not an issue for our use case).

@timsehn timsehn added enhancement New feature or request sql Issue with SQL labels Mar 21, 2024
@timsehn
Copy link
Contributor

timsehn commented Mar 21, 2024

You are correct, we don't support this variable and have not thought deeply about how Dolt works under concurrency and auto_increment.

@jycor will do some research and scope the work required to support these modes.

It's possible Dolt's transaction model is already "traditional" or "conservative" and we just need to populate the variable which would be an easy fix. If not, it may be a bit more work to support the exact MySQL behavior.

We'll try and scope this today.

@jycor
Copy link
Contributor

jycor commented Mar 21, 2024

Based off my understanding of the MySQL Docs, Dolt's current transaction model, and Dolt's auto_increment behavior, I think we currently do Interleaved, so innodb_autoinc_lock_mode=2.

I will get started on adding this variable, should take maybe a day or two.
I'll see how difficult it would be to support traditional locking.

@zachmu
Copy link
Member

zachmu commented Mar 27, 2024

@arvidfm can you comment on whether having the innodb_autoinc_lock_mode report the correct value is sufficient, or if you are blocked on getting the other two modes to work the same as in MySQL?

@arvidfm
Copy link
Author

arvidfm commented Mar 27, 2024

@zachmu In our case we do specifically need to be able to set it to consecutive (or traditional) as we rely on it for bulk inserts; it allows us to correctly calculate the IDs of inserted items without having to either run another query or insert the items one by one, using the method described e.g. here (as MySQL unfortunately doesn't supporting a RETURNING clause, unlike e.g. MariaDB). And we do a lot of bulk inserts.

We don't however require it to be changeable at runtime, so a static configuration value + the ability to query the variable to ensure that it's set correctly would be enough for our purposes

@zachmu
Copy link
Member

zachmu commented Mar 27, 2024

Thanks for the confirmation. We'll get started on implementing the different modes.

@zachmu zachmu changed the title innodb_autoinc_lock_mode global variable not supported Support for traditional and consecutive behavior for innodb_autoinc_lock_mode Mar 27, 2024
@zachmu
Copy link
Member

zachmu commented Mar 28, 2024

@nicktobey is going to start on this today, we should have this out soon.

@nicktobey
Copy link
Contributor

@arvidfm: to clarify, do you require both traditional and consecutive lock modes, or do you just need either lock mode to be supported?

@arvidfm
Copy link
Author

arvidfm commented Mar 28, 2024

@nicktobey Either is fine as far as we're concerned. Probably consecutive would be preferable since it can avoid locking the table for the whole duration of the statement in some cases, as described in the documentation, so it should scale better generally. The difference in semantics for "mixed-mode" inserts is not relevant to us.

@bpf120
Copy link

bpf120 commented Apr 1, 2024

@arvidfm , it would be great to learn about your Dolt use case too. Feel free to join our Discord or send me an email if you'd like to share.

@nicktobey
Copy link
Contributor

@arvidfm

Thank you for your patience while we investigated and implemented this issue. It should be live in the newest patch release. Feel free to try it out and let us know if you encounter any issues with it!

We ending up refactoring some of our mutex logic in order to properly support this, in ways that should improve performance for all lock modes. So that's exciting too!

If you have any other requests, comments, or concerns, please let us know and we'll get right on them!

@arvidfm
Copy link
Author

arvidfm commented Apr 12, 2024

@nicktobey Wow, that's amazing, thank you so much! I'll definitely give it a spin when I can. Also, sorry I didn't manage to find the time to dig up my Discord account to share our use case, is that something you're still interested in?

@bpf120
Copy link

bpf120 commented Apr 12, 2024

@arvidfm, we are still interested in your use case. :) You can email me at brianf@dolthub.com if that's easier than Discord.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
customer issue enhancement New feature or request sql Issue with SQL
Projects
None yet
7 participants