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

sql: TestDropColumnAfterMutations failed #76843

Closed
cockroach-teamcity opened this issue Feb 21, 2022 · 5 comments · Fixed by #107489
Closed

sql: TestDropColumnAfterMutations failed #76843

cockroach-teamcity opened this issue Feb 21, 2022 · 5 comments · Fixed by #107489
Assignees
Labels
branch-master Failures and bugs on the master branch. C-test-failure Broken test (automatically or manually discovered). O-robot Originated from a bot. skipped-test T-sql-foundations SQL Foundations Team (formerly SQL Schema + SQL Sessions)

Comments

@cockroach-teamcity
Copy link
Member

cockroach-teamcity commented Feb 21, 2022

sql.TestDropColumnAfterMutations failed with artifacts on master @ 1a89121d67fa02d5c674766ea01d3c78455fd28b:

=== RUN   TestDropColumnAfterMutations
    test_log_scope.go:79: test logs captured to: /artifacts/tmp/_tmp/233e6aa31ff7ad29c896f0479cb5d0c4/logTestDropColumnAfterMutations2123654712
    test_log_scope.go:80: use -show-logs to present logs inline
=== CONT  TestDropColumnAfterMutations
    schema_changer_test.go:7225: -- test log scope end --
    schema_changer_test.go:7225: Leaked goroutine: goroutine 78175278 [IO wait]:
        internal/poll.runtime_pollWait(0x7f829325bdc8, 0x72)
        	GOROOT/src/runtime/netpoll.go:234 +0x89
        internal/poll.(*pollDesc).wait(0xc018402600, 0xc00ce9b0a0, 0x0)
        	GOROOT/src/internal/poll/fd_poll_runtime.go:84 +0x32
        internal/poll.(*pollDesc).waitRead(...)
        	GOROOT/src/internal/poll/fd_poll_runtime.go:89
        internal/poll.(*FD).Read(0xc018402600, {0xc00ce9b0a0, 0x1, 0x200})
        	GOROOT/src/internal/poll/fd_unix.go:167 +0x25a
        net.(*netFD).Read(0xc018402600, {0xc00ce9b0a0, 0x7b2845, 0xc018402600})
        	GOROOT/src/net/fd_posix.go:56 +0x29
        net.(*conn).Read(0xc0141783a0, {0xc00ce9b0a0, 0x7c70ee, 0xc0141783a0})
        	GOROOT/src/net/net.go:183 +0x45
        io.ReadAtLeast({0x5991c20, 0xc0141783a0}, {0xc00ce9b0a0, 0x1, 0x200}, 0x1)
        	GOROOT/src/io/io.go:328 +0x9a
        io.ReadFull(...)
        	GOROOT/src/io/io.go:347
        github.com/lib/pq.(*conn).ssl(0xc00ce9b080, 0xc0001b0020)
        	github.com/lib/pq/external/com_github_lib_pq/conn.go:1087 +0x145
        github.com/lib/pq.(*Connector).open(0xc01a4b6978, {0x5a24bf8, 0xc0001b0020})
        	github.com/lib/pq/external/com_github_lib_pq/conn.go:327 +0x2b8
        github.com/lib/pq.DialOpen({0x59af070, 0xc01850bbc0}, {0xc01ddf74a0, 0xc000193fb0})
        	github.com/lib/pq/external/com_github_lib_pq/conn.go:291 +0x7a
        github.com/lib/pq.Open(...)
        	github.com/lib/pq/external/com_github_lib_pq/conn.go:281
        github.com/lib/pq.Driver.Open({}, {0xc01ddf74a0, 0x15f})
        	github.com/lib/pq/external/com_github_lib_pq/conn.go:54 +0x7a
        database/sql.dsnConnector.Connect(...)
        	GOROOT/src/database/sql/sql.go:761
        database/sql.(*DB).conn(0xc010436ea0, {0x5a24bf8, 0xc0001b0020}, 0x1)
        	GOROOT/src/database/sql/sql.go:1364 +0x7ac
        database/sql.(*DB).exec(0xc000739718, {0x5a24bf8, 0xc0001b0020}, {0x4839a95, 0xca}, {0x0, 0x0, 0x0}, 0x0)
        	GOROOT/src/database/sql/sql.go:1622 +0x5d
        database/sql.(*DB).ExecContext(0x10423e0, {0x5a24bf8, 0xc0001b0020}, {0x4839a95, 0xca}, {0x0, 0x0, 0x0})
        	GOROOT/src/database/sql/sql.go:1601 +0xed
        github.com/cockroachdb/cockroach/pkg/sql_test.TestDropColumnAfterMutations.func5.1()
        	github.com/cockroachdb/cockroach/pkg/sql_test/pkg/sql/schema_changer_test.go:7148 +0x48
        created by github.com/cockroachdb/cockroach/pkg/sql_test.TestDropColumnAfterMutations.func5
        	github.com/cockroachdb/cockroach/pkg/sql_test/pkg/sql/schema_changer_test.go:7143 +0x255
--- FAIL: TestDropColumnAfterMutations (15.44s)
Help

See also: How To Investigate a Go Test Failure (internal)
Parameters in this failure:

  • TAGS=bazel,gss,deadlock

/cc @cockroachdb/sql-schema

This test on roachdash | Improve this report!

Jira issue: CRDB-13300

@cockroach-teamcity cockroach-teamcity added branch-master Failures and bugs on the master branch. C-test-failure Broken test (automatically or manually discovered). O-robot Originated from a bot. labels Feb 21, 2022
@blathers-crl blathers-crl bot added the T-sql-schema-deprecated Use T-sql-foundations instead label Feb 21, 2022
@ajwerner
Copy link
Contributor

ajwerner commented Mar 1, 2022

This is odd.

ajwerner added a commit to ajwerner/cockroach that referenced this issue Mar 1, 2022
I have no clue what is going on in cockroachdb#76843 but this test was fooling itself
regarding the existence of separate connections.

Release justification: non-production code changes

Release note: None
craig bot pushed a commit that referenced this issue Mar 7, 2022
77208: sql: update test that was fooling itself r=ajwerner a=ajwerner

I have no clue what is going on in #76843 but this test was fooling itself
regarding the existence of separate connections.

Release justification: non-production code changes

Release note: None

77220: sql/contention/txnidcache: reuse blocks in list, account for space r=maryliag,ajwerner a=ajwerner

This change does two things to the txnidcache:
1) It accounts for the space used by the fifo eviction list. Previously
   we'd use more than double the intended space. We should probably also
   subtrace out the size of the buffers we're currently filling and the
   channel we use to communicate them, but I'll leave that for later.
2) It stops trying to compact the blocks. Compacting the blocks ends up
   being a good deal of overhead because we have to copy across every
   single message. Instead we can just append the block directly to the
   list. This does have the hazard of wasting a lot of space when the
   blocks are sparse. However, if the blocks are sparse, we know that the
   throughput is low, so it's fine.

Resolves #76738

Release justification: bug fixes and low-risk updates to new functionality

Release note: None

77363: sql/delegate: avoid extra string->int parsing r=otan a=rafiss

Release justification: low risk improvement
Release note: None

77438: ui: Remove stray parenthesis in Jobs page r=jocrl a=jocrl

Addresses #77440.

This commit fixes the stray parenthesis at the end of the duration time for a
succeeded job. The parenthesis had been introduced in
#76691 and the 21.2 backport
#73624.

Before:
![image](https://user-images.githubusercontent.com/91907326/157065776-456c8f7d-1958-4192-b38d-dcb40432cf9d.png)

After:
![image](https://user-images.githubusercontent.com/91907326/157065785-e3f2db6a-67d1-4ae3-87cb-df71dccf0e5f.png)


Release note (ui): Remove stray parenthesis at the end of the duration time for
a succeeded job. It had been accidentally introduced to unreleased master and a
21.2 backport.

Release justification: Category 2, UI bug fix

Co-authored-by: Andrew Werner <[email protected]>
Co-authored-by: Rafi Shamim <[email protected]>
Co-authored-by: Josephine Lee <[email protected]>
@ajwerner

This comment was marked as outdated.

@ajwerner ajwerner closed this as completed Mar 8, 2022
@tbg tbg reopened this Apr 27, 2022
tbg added a commit to tbg/cockroach that referenced this issue Apr 27, 2022
Refs: cockroachdb#76843

Reason: flaky test

Generated by bin/skip-test.

Release justification: non-production code changes

Release note: None
@ajwerner ajwerner self-assigned this Apr 27, 2022
craig bot pushed a commit that referenced this issue Apr 27, 2022
80634: sql: skip TestDropColumnAfterMutations r=ajwerner a=tbg

Refs: #76843

Reason: flaky test

Generated by bin/skip-test.

Release justification: non-production code changes

Release note: None

Co-authored-by: Tobias Grieger <[email protected]>
@ajwerner
Copy link
Contributor

ajwerner commented Jul 5, 2022

The locking in this test is not always right. I think there are cases where we don't clean everything up.

@ajwerner ajwerner removed their assignment Jul 5, 2022
@tbg
Copy link
Member

tbg commented Jul 6, 2022

Just FYI this test is skipped

@exalate-issue-sync exalate-issue-sync bot added the T-sql-foundations SQL Foundations Team (formerly SQL Schema + SQL Sessions) label May 10, 2023
craig bot pushed a commit that referenced this issue Jul 27, 2023
107474: cli/zip: emit SQL table data using TSV by default r=abarganier a=knz

Fixes #107473.
Epic: CRDB-28893

This is a partial revert of 35738d4.

It changes the default value of the `--format` flag back from JSON to TSV.

Release note (backward-incompatible change):
THIS RELEASE NOTE CANCELS THE CORRESPONDING PREVIOUS BACKWARD-INCOMPATIBLE CHANGE. New behavior, compatible with previous versions of CockroachDB: the command `cockroach debug zip` stores data retrieved from SQL tables in the remote cluster using the TSV format by default.

Release note (cli change): The default value of the `--format` parameter to `cockroach debug zip` is `tsv`, like other CLI commands that can extract SQL data.

107489: sql: Delete invalid TestDropColumnAfterMutations test r=rafiss a=rimadeodhar

This test checks the functionality for the following sequence of events:
1. A txn adds a constraint to a column on a table.
2. A separate txn drops the column.

However, this interaction between the two txns has been made explicit by the PR #92289. Since this PR, step (2) will fail if the constraint in step (1) is in the process of being added. As a result, the elaborate set up and sequence of events being tested in TestDropColumnAfterMutations is no longer necessary.

Release note: none
Epic: none
Fixes: #76843

107664: authors: add Angela Dietz to authors r=angeladietz a=angeladietz

Release note: None
Epic: None

107666: server: fix a race in tenant creation r=knz a=lidorcarmel

Previously, scanTenantsForRunnableServices() was not holding the mutex when SELECTing for the existing tenant names, which means that the following may happen:
- scanTenantsForRunnableServices() sees that only the system tenant exists
- createServerEntryLocked() then adds another tenant while holding the mutex
- scanTenantsForRunnableServices() takes the lock and stops the tenant that was just created because only the system tenant should be alive (which is wrong)

This patch changes scanTenantsForRunnableServices() to take the mutex before SELECTing for the existing tenants in order to avoid the race.

Epic: none
Fixes: #107434
Fixes: #107343
Fixes: #107154

Release note: None

107673: opt: remove Metadata.AllUserDefinedFunctions r=mgartner a=mgartner

The metadata method `AllUserDefinedFunctions` has been replaced with a
new function `HasUserDefinedFunctions` which provides a simpler API
without exposing the underlying UDF dependency map. The map is still
available outside of the opt package via the `TestingUDFDeps` method
which is designed for testing use only.

Epic: None

Release note: None


107714: roachtest: add warning to redacted github issue r=mgartner a=mgartner

Epic: None

Release note: None

107716: ui: extend search logic on insights page r=koorosh a=koorosh

This change extends the number of fields where
search is applied (instead of single transaction/
statement execution ID field).
It makes possible to search for any available ID
in Txn or statement insight.

Release note (ui change): search is performed on all ID fields of transaction and statement insights.

Resolves: #107253

Demo:

https://github.com/cockroachdb/cockroach/assets/3106437/7fb56720-3ab2-4be4-9500-457707f6f01d



Co-authored-by: Raphael 'kena' Poss <[email protected]>
Co-authored-by: rimadeodhar <[email protected]>
Co-authored-by: Angela Dietz <[email protected]>
Co-authored-by: Lidor Carmel <[email protected]>
Co-authored-by: Marcus Gartner <[email protected]>
Co-authored-by: Andrii Vorobiov <[email protected]>
@craig craig bot closed this as completed in 7833c65 Jul 27, 2023
blathers-crl bot pushed a commit that referenced this issue Jul 27, 2023
This test checks the functionality for the following sequence
of events:
1. A txn adds a constraint to a column on a table.
2. A separate txn drops the column.

However, this interaction between the two txns has been
made explicit by the PR #92289.
Since this PR, step (2) will fail if the constraint in step (1)
is in the process of being added. As a result, the elaborate
set up and sequence of events being tested in TestDropColumnAfterMutations
is no longer necessary.

Release note: none
Epic: none
Fixes: #76843
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
branch-master Failures and bugs on the master branch. C-test-failure Broken test (automatically or manually discovered). O-robot Originated from a bot. skipped-test T-sql-foundations SQL Foundations Team (formerly SQL Schema + SQL Sessions)
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants