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

mysql_query check mode using transaction #613

Open
drzraf opened this issue Feb 7, 2024 · 4 comments
Open

mysql_query check mode using transaction #613

drzraf opened this issue Feb 7, 2024 · 4 comments

Comments

@drzraf
Copy link

drzraf commented Feb 7, 2024

SUMMARY

mysql_query should support check mode using start transaction

ISSUE TYPE
  • Feature Idea
COMPONENT NAME

mysql_query

ADDITIONAL INFORMATION

Would be useful to check queries (eg: UPDATE/INSERT) beforehand

START TRANSACTION;
UPDATE foobar SET abc='xyz';
ROLLBACK;
Quick and dirty proposal:

if isinstance(query, str):
query = [query]

 if check_mode:
    query = ["START TRANSACTION", *query, "ROLLBACK"]
Current workaround
    - mysql_query:
        login_db: db
        query:
          - START TRANSACTION
          - "UPDATE foobar SET abc='xyz'"
          - ROLLBACK

#  "query_result": [[], [], []], "rowcount": [0, 42, 0]
@Andersson007
Copy link
Collaborator

@drzraf hello, thanks for opening the issue, and at the first glance it sounds reasonable.

  • We should be sure that this syntax is supported with all kinds of queries. I know that in postgres at least all or some DDL queries cannot be run in a transaction. Though don't remember the details and don't know how it works with MySQL. Are we sure?:)
  • Also it feels like a major change to me as the task with the workaround in the description will probably fail after the changes arrive.

I see @markuman seems to like the idea, thanks
Any other opinions?

@laurent-indermuehle
Copy link
Collaborator

I'm not totally comfortable with the idea. I see ansible check_mode as something really safe. But here, it would create a real transaction on a live server!

If we are 99% sure it won't create any deadlock or other nasty things in the database server, then yeah, this feature seems cool! But I'm failing to imagine a use case.

@drzraf can you tell us more please? What do you want to test with the check_mode? That you have the necessary privileges? That the table exists? In what situation? What kind of tables do you edit from Ansible?

@drzraf
Copy link
Author

drzraf commented Mar 16, 2024

I've several mysqld running with similar DB structures and use ansible not only for provisioning but as my go-to command multiplexer. (Using playbooks or ansible -m instead of parallel + ssh or other dedicated tools) and it's particularly handy for read-only mysql commands

I encountered situation where an INSERT / UPDATE / search-replace needs to be issued on multiple instances. That's where having a dry-run mode based on SQL transactions is handy: we know immediately how many rows would be updated on each instance and whether it's consistent with the expectations/previous local-testing.

It's not bullet-proof in the sense that it won't avoid all possible mistakes.
For example UPDATE text = REPLACE(text, "foo(bar?)", "\1") would actually show the correct number of rows updated, but replacing the field with 1 instead of bar... because Ansible command needed another level of escaping (\\\\1) ((which, IMHO would be worth a warning!)).

... but supporting check-mode will always be an improvement over the current situation where the admin has no clue about the result of the request in -prod before it's run and he's gonna run it anyway... "blindly".

99% read-only certainty ?, likely.

  1. It won't help if the query contains a "COMMIT".
  2. Normally, a "START TRANSACTION" wouldn't be committed when the connection is closed, even if no "ROLLBACK" was issued but double-checking this assumption could be desirable.
  3. If the query already contains a TRANSACTION or MySQL disabled transactional mode (is that even possible?) weird things may happen
  4. Some other edge-cases may exists but more implausible than the above.

Disable auto-reconnect may be desirable:

   Disabling mysql Auto-Reconnect
       If the mysql client loses its connection to the server while sending a
       statement, it immediately and automatically tries to reconnect once to
       the server and send the statement again. However, even if mysql
       succeeds in reconnecting, your first connection has ended and all your
       previous session objects and settings are lost: temporary tables, the
       autocommit mode, and user-defined and session variables. Also, any
       current transaction rolls back. This behavior may be dangerous for
       you, as in the following example where the server was shut down and
       restarted between the first and second statements without you knowing
       it:

We wouldn't want a statement to be run outside a transaction just because a disconnection happened right after "START TRANSACTION" but before the r/w statement.

@laurent-indermuehle
Copy link
Collaborator

@drzraf thank you for your very descriptive explanation! It could serve as a requirement for writing the units and integrations tests!
If you want to get started on this PR, I'll be happy to help you when you have questions (both here or on Matrix)

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

No branches or pull requests

3 participants