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

teamcity: failed test: TestLogic #38062

Closed
cockroach-teamcity opened this issue Jun 6, 2019 · 4 comments · Fixed by #38397
Closed

teamcity: failed test: TestLogic #38062

cockroach-teamcity opened this issue Jun 6, 2019 · 4 comments · Fixed by #38397
Assignees
@knz
Labels
C-test-failure Broken test (automatically or manually discovered). O-robot Originated from a bot.
Milestone
19.2

Comments

@cockroach-teamcity
Copy link
Member

The following tests appear to have failed on master (test): TestLogic/fakedist: TestLogic/fakedist/crdb_internal, TestLogic/fakedist, TestLogic/fakedist: TestLogic/fakedist/crdb_internal: TestLogic/fakedist/crdb_internal/max_retry_counter, TestLogic

You may want to check for open issues.

#1327655:

TestLogic/fakedist: TestLogic/fakedist/crdb_internal
--- FAIL: test/TestLogic: TestLogic/fakedist: TestLogic/fakedist/crdb_internal (2.170s)

------- Stdout: -------
=== PAUSE TestLogic/fakedist/crdb_internal



TestLogic
--- FAIL: test/TestLogic (428.390s)

------- Stdout: -------
    test_log_scope.go:79: test logs captured to: /tmp/logTestLogic244295760
    test_log_scope.go:60: use -show-logs to present logs inline



TestLogic/fakedist: TestLogic/fakedist/crdb_internal: TestLogic/fakedist/crdb_internal/max_retry_counter
--- FAIL: test/TestLogic: TestLogic/fakedist: TestLogic/fakedist/crdb_internal: TestLogic/fakedist/crdb_internal/max_retry_counter (0.020s)

------- Stdout: -------
--- done: testdata/logic_test/crdb_internal with config fakedist: 78 tests, 1 failures
=== CONT  TestLogic/fakedist/collatedstring_uniqueindex2
=== CONT  TestLogic/fakedist/inet
                logic.go:2336: 
                     
                    testdata/logic_test/crdb_internal:434: SELECT key, (max_retries > 1), flags LIKE '!%' AS f
                      FROM crdb_internal.node_statement_statistics
                     WHERE application_name = 'test_max_retry'
                    ORDER BY key, f
                    expected:
                        SELECT crdb_internal.force_retry(_)  true  false
                        SELECT crdb_internal.force_retry(_)  true  true
                        SET application_name = DEFAULT       false false
                    but found (query options: "") :
                        SELECT crdb_internal.force_retry(_)  false  false
                        SELECT crdb_internal.force_retry(_)  false  true
                        SET application_name = DEFAULT       false  false
                        
                    



TestLogic/fakedist
--- FAIL: test/TestLogic: TestLogic/fakedist (0.850s)

Please assign, take a look and update the issue accordingly.
@cockroach-teamcity cockroach-teamcity added this to the 19.2 milestone Jun 6, 2019
@cockroach-teamcity cockroach-teamcity added C-test-failure Broken test (automatically or manually discovered). O-robot Originated from a bot. labels Jun 6, 2019
@tbg
Copy link
Member

tbg commented Jun 20, 2019

Also failed my PR build in https://teamcity.cockroachdb.com/viewLog.html?buildId=1351565&buildTypeId=Cockroach_UnitTests. friendly ping @jordanlewis

@tbg tbg mentioned this issue Jun 20, 2019
Merged
@jordanlewis
Copy link
Member

@knz could you take a look at this? I think you tried to fix it in #38045 but it doesn't seem to have completely fixed the problem.

@jordanlewis jordanlewis assigned knz and unassigned jordanlewis Jun 24, 2019
@knz
Copy link
Contributor

knz commented Jun 25, 2019

this is a new occurrence of it, this time related to additional retries caused by the fakedist executor. Either I'll restric the test to only run locally, or figure out how to limit the retries otherwise.

@knz
Copy link
Contributor

knz commented Jun 25, 2019

more specifically, the test appears to run too slow to trigger the retry.
I am surprised that the other txn tests are not failing for the same reason.

OTOH the manual_retry tests extend the txn duration to a full second, with a 500ms delay given to force_retry().

@andreimatei what do you think? Should we just extend the delay?

@knz knz mentioned this issue Jun 25, 2019
Merged
craig bot pushed a commit that referenced this issue Jul 8, 2019
@nvanbenschoten @knz @rohany
Merge #37199 #38397 #38654 #38725
3798cc8 
37199: storage: propagate errors from contentionQueue, catch stalls in roachtest r=nvanbenschoten a=nvanbenschoten

Informs #36089.

The PR is split into a series of commits. The first fixes part of a bug that was causing #36089  to fail (thanks to #36748) and the second improves the test to have a more obvious failure condition for this class of bug in the future. The third, fifth, and sixth clean up code. Finally, the fourth fixes another bug that could cause issues with #36089.

Before the first commit, requests could get stuck repeatedly attempting to push a transaction only to repeatedly find that they themselves were already aborted. The error would not propagate up to the transaction coordinator and the request would get stuck. This commit fixes this behavior by correctly propagating errors observed by the `contentionQueue`.

The second commit bumps the TxnLivenessThreshold for clusters running `kv/contention/nodes=4` to 10 minutes. This is sufficiently large such that if at any point a transaction is abandoned then all other transactions will begin waiting for it, throughput will drop to 0 for 10 straight minutes, and the test will fail to achieve its minimum QPS requirement.

The fourth commit instructs pushers in the `txnwait.Queue` to inform all other pushers that are waiting for the same transaction when it observes an ABORTED transaction. I never saw this cause issues with #36089, but it seems very possible that it could given frequent tscache rotations.

38397: sql: deflake TestLogic//crdb_internal/max_retry_counter r=knz a=knz

Fixes #38062.

Release note: None

38654: exec: Handle NULLS in TopK sorter r=rohany a=rohany

This commit fixes NULLs in the TopK sorter by avoiding use
of the vec copy method, which has a bug. Instead, we add
a set method to the vec comparator, and use the templatized
comparator to perform the sets that the TopK sorter needs.

To facilitate this, we add an UnsetNull method to the Nulls
object. However, use of this method results in HasNull()
maybe returning true even if the vector doesn't have nulls.
This behavior already occurs when selection vectors are used.
Based on discussions with @solongordon and @asubiotto, this behavior
is OK, and future PR's will attempt to make this behavior better, and address
the bugs within the Vec Copy method.

38725: cli/dump: more clearly inform the user upon tables with no visible columns r=knz a=knz

Informs #37768.
Informs #28948.
This is coming up quite often on support, lately again on gitter and forum https://forum.cockroachlabs.com/t/error-while-dumping-core-backup/2901/3.

This PR aims to lessen the burden on support and propose a clear "next action" for the user.

Before:

```
kena@kenax ~/cockroach % ./cockroach dump --insecure defaultdb
CREATE TABLE t (,
        FAMILY "primary" (rowid)
);
Error: pq: at or near "from": syntax error
Failed running "dump"
```

After:

```
kena@kenax ~/cockroach % ./cockroach dump --insecure defaultdb
CREATE TABLE t (,
        FAMILY "primary" (rowid)
);
Error: table "defaultdb.public.t" has no visible columns
HINT: To proceed with the dump, either omit this table from the list of tables to dump, drop the table, or add some visible columns.
--
See: #37768
Failed running "dump"
```

Release note (cli change): `cockroach dump` will now more clearly
refer to issue #37768 when it encounters a table with no visible
columns, which (currently) cannot be dumped successfully.

Co-authored-by: Nathan VanBenschoten <nvanbenschoten@gmail.com>
Co-authored-by: Raphael 'kena' Poss <knz@cockroachlabs.com>
Co-authored-by: Rohan Yadav <rohany@alumni.cmu.edu>
@craig craig bot closed this as completed in #38397 Jul 8, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@knz knz

Labels
C-test-failure Broken test (automatically or manually discovered). O-robot Originated from a bot.
Projects
None yet
Milestone
19.2
Development

Successfully merging a pull request may close this issue.

sql: deflake TestLogic//crdb_internal/max_retry_counter knz/cockroach
4 participants
@jordanlewis @knz @tbg @cockroach-teamcity