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.

Sign up for GitHub

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

Jump to bottom

circleci: failed tests (13586): TestConcurrentIncrements #4752

Closed
tbg opened this issue Mar 1, 2016 · 1 comment
Closed

circleci: failed tests (13586): TestConcurrentIncrements #4752

tbg opened this issue Mar 1, 2016 · 1 comment
Assignees
@andreimatei

Comments

@tbg
Copy link
Member

tbg commented Mar 1, 2016

The following test appears to have failed:

#13586:

I160301 19:22:40.132965 client/db.go:491  failed batch: retry txn "sql/executor.go:597 sql implicit: false" id=25780b53 key=/Table/51/1/1/2/1 rw=true pri=0.04612776 iso=SERIALIZABLE stat=PENDING epo=1 ts=1456860160.128619739,1 orig=1456860160.128619739,1 max=1456860160.167489342,0
I160301 19:22:40.133518 storage/replica_command.go:469  auto-gc'ed 25780b53 (1 intents)
E160301 19:22:40.133831 client/txn.go:346  failure aborting transaction: client/txn.go:513: attempting to use ABORTED transaction; abort caused by: retry txn "sql/executor.go:597 sql implicit: false" id=25780b53 key=/Table/51/1/1/2/1 rw=true pri=0.04612776 iso=SERIALIZABLE stat=PENDING epo=1 ts=1456860160.128619739,1 orig=1456860160.128619739,1 max=1456860160.167489342,0
I160301 19:22:40.149103 storage/replica_command.go:469  auto-gc'ed a1227219 (1 intents)
I160301 19:22:40.153350 storage/engine/rocksdb.go:137  closing in-memory rocksdb instance
--- FAIL: TestConcurrentIncrements (0.63s)
    driver_test.go:481: current transaction is aborted, commands ignored until end of transaction block
FAIL
FAIL    github.com/cockroachdb/cockroach/sql/driver 4.649s
=== RUN   TestEval
--- PASS: TestEval (0.01s)
=== RUN   TestEvalError
--- PASS: TestEvalError (0.00s)
=== RUN   TestEvalComparisonExprCaching
--- PASS: TestEvalComparisonExprCaching (0.00s)
=== RUN   TestSimilarEscape
--- PASS: TestSimilarEscape (0.00s)
=== RUN   TestQualifiedNameString
--- PASS: TestQualifiedNameString (0.00s)

Please assign, take a look and update the issue accordingly.
@tbg tbg added the C-test-failure Broken test (automatically or manually discovered). label Mar 1, 2016
@tamird
Copy link
Contributor

tamird commented Mar 1, 2016

#4576 looks like a likely suspect. @andreimatei can you take a look?

@tamird tamird added test-disabled and removed C-test-failure Broken test (automatically or manually discovered). labels Mar 2, 2016
@tamird tamird reopened this Mar 2, 2016
@andreimatei andreimatei mentioned this issue Mar 2, 2016
Merged
andreimatei added a commit to andreimatei/cockroach that referenced this issue Mar 3, 2016
@andreimatei
sql: match postgres behavior in handling of errors on COMMIT
d0b04cd 
The txn needs to not remain in the aborted state if a COMMIT is executed
and errors, as there's no way for the client to recover (e.g. in go you
can't use a Txn object after doing Txn.Commit()). As before this change,
an error is still returned to the client.

Keep in mind that statements in the same batch coming after a failed
COMMIT are still not executed; that's in line with the principle of not
running any statements after an error since we can't return results for
them.

The change pushes the responsibility of updating txnState down from
execStmtsInCurrentTxn() to execStmtInCurrentTxn(). The actual change in
behavior comes from setting txnState.aborted = false in the `txnDone`
case. Before, the error would take precedence and txnState.aborted would
be set to true.

Fixes cockroachdb#4752
andreimatei added a commit to andreimatei/cockroach that referenced this issue Mar 3, 2016
@andreimatei
sql: match postgres behavior in handling of errors on COMMIT
e3af593 
The txn needs to not remain in the aborted state if a COMMIT is executed
and errors, as there's no way for the client to recover (e.g. in go you
can't use a Txn object after doing Txn.Commit()). As before this change,
an error is still returned to the client.

Keep in mind that statements in the same batch coming after a failed
COMMIT are still not executed; that's in line with the principle of not
running any statements after an error since we can't return results for
them.

The change pushes the responsibility of updating txnState down from
execStmtsInCurrentTxn() to execStmtInCurrentTxn(). The actual change in
behavior comes from setting txnState.aborted = false in the `txnDone`
case. Before, the error would take precedence and txnState.aborted would
be set to true.

Fixes cockroachdb#4752
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@andreimatei andreimatei

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
3 participants
@andreimatei @tamird @tbg