docs(spanner): fix the result handling after BufferWrite

[sryoya]

Fixes #3802

[olavloite]

Good catch.

Transaction.BufferWrite will however normally not throw any errors (unless you have manually closed the transaction) as it only buffers the mutation in an in-memory data structure. So this would not affect transaction retries if you were to implement your own transaction functions using this sample. Any Aborted error would be returned by the Commit that is executed after the transaction function has returned.

[olavloite]

nit: could we move this line to after the comment. It does not feel right to have a comment after the return statement of a function.

[sryoya]

Yup, I actually I wondered if it's right.
Then, fixed this with the similar code I referred to.

64546a2

[sryoya]

@olavloite Thanks for your explanation.
As you say, BufferWrite doesn't seem to actually return an error.
But, One thing I've experienced is that since I added handling for error from the BufferWrite in my company's application , the failure rate of writes to Spanner has clearly decreased.
I'm not sure if it actually affected, but from that, I guessed the error from BufferWrite should be handled.
Also, I find the begin method, which mutates t.state, misses the Mutex lock. I guess it's irrelevant, but created the another PR for now #3841 (As commented in the description, if it's intentional, let me know that)

unless you have manually closed the transaction

Btw, how can we do this? I'm just curisous.

[olavloite]

Btw, how can we do this? I'm just curisous.

You cannot do it with the normal transaction runner, but you could do it if you use statement-based transactions, by calling Rollback on the returned transaction. Using statement-based transactions is however only recommended for advanced usage (like writing a SQL driver), as it requires you to handle all retries of aborted transactions and other possible failures manually.

But, One thing I've experienced is that since I added handling for error from the BufferWrite in my company's application , the failure rate of writes to Spanner has clearly decreased.

I'm pretty sure that that is either coincidence or caused by a different change that was applied at the same time, as the BufferWrite function will never return a retryable error. It would therefore never affect the number of read/write transactions that need to be retried.

[gcf-merge-on-green]

Merge-on-green attempted to merge your PR for 6 hours, but it was not mergeable because either one of your required status checks failed, one of your required reviews was not approved, or there is a do not merge label. Learn more about your required status checks here: https://help.github.com/en/github/administering-a-repository/enabling-required-status-checks. You can remove and reapply the label to re-run the bot.

[olavloite]

@tbpg Would you be able to merge this one? The integration tests are not running as the PR comes from a fork. They are green however (and not really relevant, as it is only a documentation change).