Update SQLAlchemy docs to remove incorrect info that SAVEPOINT is faster

[rmloveland]

In the Build a Python app with SQLAlchemy docs we say re: the run_transaction() wrapper method that:

It will run your transactions to completion much faster than a naive implementation that created a fresh transaction after every retry error. Because of the way the CockroachDB dialect's driver structures the transaction attempts (using a SAVEPOINT statement under the hood, which has a slightly different meaning in CockroachDB than in some other databases), the server is able to preserve some information about the previously attempted transactions to allow subsequent retries to complete more easily.

This is not actually correct. Using an application-level retry loop that does not use SAVEPOINT is basically fine most of the time, and performs well if you add exponential backoff.

For context see the discussion in this mailing list thread, and also cockroachdb/cockroach#5935

[vy-ton]

@ericharmeling @vy-ton Note that we should update this before the SQLAlchemy relaunch

[vy-ton]

This is not actually correct. Using an application-level retry loop that does not use SAVEPOINT is basically fine most of the time, and performs well if you add exponential backoff.

Not sure I understand the desired outcome of this issue. My understanding of the current docs is that we do not recommend that SAVEPOINT is faster and encourage everyone to use the run_transaction() wrapper provided in CRDB's dialect.

Are we saying that users should use an application-level retry loop that does not use SAVEPOINT with exponential backoff instead?

cc @rafiss to double-check the recommendation

[rafiss]

Not sure I understand the desired outcome of this issue.

It seems that the desired outcome is to remove this paragraph from the docs. We can keep recommending use of run_transaction, but we might as well remove this paragraph if it's something that is still up for debate.
"It will run your transactions to completion much faster than a naive implementation that created a fresh transaction after every retry error. Because of the way the CockroachDB dialect's driver structures the transaction attempts (using a SAVEPOINT statement under the hood, which has a slightly different meaning in CockroachDB than in some other databases), the server is able to preserve some information about the previously attempted transactions to allow subsequent retries to complete more easily."

My understanding of the current docs is that we do not recommend that SAVEPOINT is faster and encourage everyone to use the run_transaction() wrapper provided in CRDB's dialect.

One point of clarification perhaps: the run_transaction() wrapper in the dialect does use SAVEPOINT.

Are we saying that users should use an application-level retry loop that does not use SAVEPOINT with exponential backoff instead?

No, I think we should keep recommending the run_transaction() wrapper.

---

I'm not familiar with this earlier linked discussion aboutSAVEPOINT performance. To get an idea of the current state of things, I think we should check with people who worked on it in the 20.1 release. If we do need an answer about the current state of the world with SAVEPOINT performance, then we could ask Andrei