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

nodejs concurrency issue #3800

Closed
zachmu opened this issue Jul 11, 2022 · 4 comments
Closed

nodejs concurrency issue #3800

zachmu opened this issue Jul 11, 2022 · 4 comments
Assignees

Comments

@zachmu
Copy link
Member

zachmu commented Jul 11, 2022

Customer report that clients cannot see writes made by other transactions using the nodejs mysql library and connection pooling. Same application worked with mariadb. We should have tests of this.

Working theory is that the library is beginning a transaction at connection time when connected to dolt, but with MariaDB it defers to statement time. We should have concurrency tests to see if this is the case.

@fulghum fulghum self-assigned this Jul 11, 2022
@fulghum
Copy link
Contributor

fulghum commented Jul 12, 2022

I've been experimenting with a repro today and found a case where there is behavioral difference between MySQL and Dolt that could explain what this customer is seeing...

I set up a test using nodejs and the mysql connector library that:

  • connects to a MySQL or Dolt server configured with autocommit turned off for all sessions
  • creates a connection pool and grabs two connections
  • on connection 1: starts an explicit transaction, inserts a row of data, and then commits
  • on connection 2: attempts to read the data just committed by connection 1

In the simple case, as expected, connection 2 is able to read the data that connection 1 committed. This is expected and identical to MySQL's behavior because connection 2 has not set its db snapshot yet.

However, MySQL and Dolt seem to have slightly different behavior on when to set the db snapshot for a transaction. From MySQL's Consistent, Nonlocking Reads documentation:

If the transaction isolation level is REPEATABLE READ (the default level), all consistent reads within the same transaction read the snapshot established by the first such read in that transaction.

In the example above, there is no "first such read" on connection 2, so it is able to see the results of connection 1's commit when we execute queries on connection 2. The difference comes from when each engine sets that snapshot. MySQL seems to set it when any data is read from a table in the transaction database. Dolt seems to set it more eagerly – when any query is executed.

To be more concrete, in the test setup above...

  • if connection 2 executes a select against the table used in connection 1's commit, then as expected, connection 2 is NOT able to see the commit because it already set its db snapshot before the commit was made. This is the same behavior for MySQL and for Dolt.
  • if connection 2 executes a non-read query (e.g. set @foo = 'bar';, select 2 from dual;), it still seems to set the db snapshot at that point, which means it is NOT able to see the commit connection 1 makes. This is a behavioral difference between Dolt and MySQL.

If the customer's application is executing any non-table-read queries in the second connection before the first connection commits, then their application would be able to see the commit from the first connection when using MariaDB/MySQL, but with Dolt, they would not be able to see the results of that commit.

@zachmu
Copy link
Member Author

zachmu commented Jul 12, 2022

This makes sense as an explanation. Do we have confirmation that the nodejs is running queries at connect time before doing any actual work?

@fulghum
Copy link
Contributor

fulghum commented Jul 12, 2022

I do not see any implicit queries being executed by the nodejs mysql library on connection. The only trace logs I see in Dolt are the two NewConnections and the two corresponding ConnectionCloseds. I don't think this is specific to nodejs and was going to repro this next with a go enginetest.

It would be nice if we could validate that this is the same bug the customer is hitting though, just to make sure we're fixing the right issue first for them.

@fulghum
Copy link
Contributor

fulghum commented Aug 31, 2022

Closing this one out for now, due to lack of repro data. Happy to reopen and continue this investigation if we can get more customer data to validate this hypothesis.

@fulghum fulghum closed this as completed Aug 31, 2022
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

2 participants