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

Do we support Transactions? #380

Open
javisantana opened this issue Nov 7, 2016 · 4 comments
Open

Do we support Transactions? #380

javisantana opened this issue Nov 7, 2016 · 4 comments
Assignees

Comments

@javisantana
Copy link
Contributor

@ztephm commented on Fri Nov 04 2016

cc @javisantana
S/B 10399055

Context

Support client wants to know if we support Transactions

Process

I tested by pasting this into browser tab:

https://stephaniemongon.carto.com/api/v2/sql?q=BEGIN; UPDATE batch_test SET name = 3 WHERE cartodb_id = 3; SAVEPOINT my_savepoint; UPDATE batch_test SET name = 4 WHERE cartodb_id = 3; ROLLBACK TO my_savepoint; UPDATE batch_test SET name = 4 WHERE cartodb_id = 4; COMMIT;&api_key=mykey

Result

Dataset was updated properly:

screen shot 2016-11-04 at 2 43 19 pm

but response in browser included 'total rows: 0':
screen shot 2016-11-04 at 2 47 13 pm

I understand 'total_rows:0' refers to rows affected by last query - but if data's being overwritten instead of creating new rows then it's proper for total rows to = 0, right?

Double-checking b/c client is interested in:

  • if we fully support Transactions
  • if one query fails would the others execute (I let him know re: Batch API but wondering re: Transactions)
@michellechandra
Copy link

hey @dgaubert - Steph is out for the next two weeks, so I am following up on this issue. Have you had a moment to review?

@javisantana
Copy link
Contributor Author

@michellechandra I'd answer we don't support transactions for the moment

@jchamberlain
Copy link
Contributor

I've looked into it a bit myself, and I believe the short answer is "no". Due to Carto's use of node-postgres, multiple statements given in a single call to the SQL API are processed as a single, implicit transaction, much like psql with the -c flag. Thus a single failure causes the whole request to be rolled back, which I believe is desirable and meets most of my needs.

Beyond this implicit transaction, transactions can't really be used in a meaningful way with Carto.

Note that parameterized queries, as I request in #382, would NOT be processed the same way by node-postgres and could no longer handle multiple statements in a single call to the SQL API, and therefore no longer take advantage of that implicit transaction.

@jchamberlain
Copy link
Contributor

Would there be any interest in adding transaction support for batch queries? Error fallbacks are nice, but not always sufficient—sometimes it's impossible to revert changes after they've been committed.

Based on my reading of the job runner and scheduler code, it seems the main difficulty is that each query in a batch is run independently. That obviously has its benefits and allows you a lot of flexibility in queuing up queries. You certainly don't want a single transaction open for days on end! However, I also see in the production.js.example config that each query is given a timeout of 12 hours. That's more than any of my queries need. Perhaps the whole job could run in that time?

I.e., We could add a transaction boolean when creating a job (defaulting to false). If true, all queries are run together in a single transaction, automatically rolling back on error. The whole job could be given the same timeout a single query normally would (12 hours), so the danger of long-open transactions remains exactly what it is now. The user just needs to know that infinite time is available only for non-transactional jobs, and that transactional jobs are limited to 12 hours.

Would there be any interest in this?

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

4 participants