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/vecindex/vecstore: TestInMemoryLock failed #136958

Closed
cockroach-teamcity opened this issue Dec 7, 2024 · 8 comments · Fixed by #137374
Closed

sql/vecindex/vecstore: TestInMemoryLock failed #136958

cockroach-teamcity opened this issue Dec 7, 2024 · 8 comments · Fixed by #137374
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. release-blocker Indicates a release-blocker. Use with branch-release-2x.x label to denote which branch is blocked. T-sql-queries SQL Queries Team

Comments

@cockroach-teamcity
Copy link
Member

cockroach-teamcity commented Dec 7, 2024

sql/vecindex/vecstore.TestInMemoryLock failed on master @ ccaed974f75032f1b908bca2d42145d21d0092ca:

=== RUN   TestInMemoryLock
    test_log_scope.go:165: test logs captured to: outputs.zip/logTestInMemoryLock3006853939
    test_log_scope.go:76: use -show-logs to present logs inline
=== RUN   TestInMemoryLock/lock_reentrancy
=== RUN   TestInMemoryLock/shared_lock_does_not_wait_for_shared_lock
POTENTIAL DEADLOCK: Recursive locking:
current goroutine 1079 lock 0xc0020f3230
pkg/sql/vecindex/vecstore/in_memory_lock.go:62 vecstore.(*inMemoryLock).AcquireShared ??? <<<<<
pkg/sql/vecindex/vecstore/in_memory_lock.go:61 vecstore.(*inMemoryLock).AcquireShared ???
GOROOT/src/sync/atomic/type.go:144 atomic.(*Uint64).Load ???

Previous place where the lock was grabbed (same goroutine)
pkg/sql/vecindex/vecstore/in_memory_lock.go:62 vecstore.(*inMemoryLock).AcquireShared ??? <<<<<
pkg/sql/vecindex/vecstore/in_memory_lock.go:61 vecstore.(*inMemoryLock).AcquireShared ???
pkg/sql/vecindex/vecstore/in_memory_store_test.go:507 vecstore.TestInMemoryLock.func2 ???

Parameters:

  • attempt=1
  • deadlock=true
  • run=1
  • shard=1
Help

See also: How To Investigate a Go Test Failure (internal)

This test on roachdash | Improve this report!

Jira issue: CRDB-45316

@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. release-blocker Indicates a release-blocker. Use with branch-release-2x.x label to denote which branch is blocked. T-sql-queries SQL Queries Team labels Dec 7, 2024
@github-project-automation github-project-automation bot moved this to Triage in SQL Queries Dec 7, 2024
@cockroach-teamcity
Copy link
Member Author

sql/vecindex/vecstore.TestInMemoryLock failed on master @ d7ea85402dc35e36c6cc35520fa91f25fd5c999d:

=== RUN   TestInMemoryLock
    test_log_scope.go:165: test logs captured to: outputs.zip/logTestInMemoryLock3584269042
    test_log_scope.go:76: use -show-logs to present logs inline
=== RUN   TestInMemoryLock/lock_reentrancy
=== RUN   TestInMemoryLock/shared_lock_does_not_wait_for_shared_lock

Parameters:

  • attempt=1
  • deadlock=true
  • run=1
  • shard=1
Help

See also: How To Investigate a Go Test Failure (internal)

This test on roachdash | Improve this report!

@cockroach-teamcity
Copy link
Member Author

sql/vecindex/vecstore.TestInMemoryLock failed on master @ 74e83fa45e549680ed0e1ce8917468701bea7794:

=== RUN   TestInMemoryLock
    test_log_scope.go:165: test logs captured to: outputs.zip/logTestInMemoryLock2989632252
    test_log_scope.go:76: use -show-logs to present logs inline
=== RUN   TestInMemoryLock/lock_reentrancy
=== RUN   TestInMemoryLock/shared_lock_does_not_wait_for_shared_lock
POTENTIAL DEADLOCK: Recursive locking:
current goroutine 1099 lock 0xc002141140
pkg/sql/vecindex/vecstore/in_memory_lock.go:62 vecstore.(*inMemoryLock).AcquireShared ??? <<<<<
pkg/sql/vecindex/vecstore/in_memory_lock.go:61 vecstore.(*inMemoryLock).AcquireShared ???
GOROOT/src/sync/atomic/type.go:144 atomic.(*Uint64).Load ???

Previous place where the lock was grabbed (same goroutine)
pkg/sql/vecindex/vecstore/in_memory_lock.go:62 vecstore.(*inMemoryLock).AcquireShared ??? <<<<<
pkg/sql/vecindex/vecstore/in_memory_lock.go:61 vecstore.(*inMemoryLock).AcquireShared ???
pkg/sql/vecindex/vecstore/in_memory_store_test.go:507 vecstore.TestInMemoryLock.func2 ???

Parameters:

  • attempt=1
  • deadlock=true
  • run=1
  • shard=1
Help

See also: How To Investigate a Go Test Failure (internal)

This test on roachdash | Improve this report!

@cockroach-teamcity
Copy link
Member Author

sql/vecindex/vecstore.TestInMemoryLock failed on master @ 5f83714a8111d0ea9ca8b34a72b2f5d97ea56b53:

=== RUN   TestInMemoryLock
    test_log_scope.go:165: test logs captured to: outputs.zip/logTestInMemoryLock3540950359
    test_log_scope.go:76: use -show-logs to present logs inline
=== RUN   TestInMemoryLock/lock_reentrancy
=== RUN   TestInMemoryLock/shared_lock_does_not_wait_for_shared_lock
POTENTIAL DEADLOCK: Recursive locking:
current goroutine 1099 lock 0xc00214d230
pkg/sql/vecindex/vecstore/in_memory_lock.go:62 vecstore.(*inMemoryLock).AcquireShared ??? <<<<<
pkg/sql/vecindex/vecstore/in_memory_lock.go:61 vecstore.(*inMemoryLock).AcquireShared ???
GOROOT/src/sync/atomic/type.go:144 atomic.(*Uint64).Load ???

Previous place where the lock was grabbed (same goroutine)
pkg/sql/vecindex/vecstore/in_memory_lock.go:62 vecstore.(*inMemoryLock).AcquireShared ??? <<<<<
pkg/sql/vecindex/vecstore/in_memory_lock.go:61 vecstore.(*inMemoryLock).AcquireShared ???
pkg/sql/vecindex/vecstore/in_memory_store_test.go:508 vecstore.TestInMemoryLock.func2 ???

Parameters:

  • attempt=1
  • deadlock=true
  • run=1
  • shard=1
Help

See also: How To Investigate a Go Test Failure (internal)

This test on roachdash | Improve this report!

@cockroach-teamcity
Copy link
Member Author

sql/vecindex/vecstore.TestInMemoryLock failed on master @ d18eb683b2759fd8814dacf0baa913f596074a17:

=== RUN   TestInMemoryLock
    test_log_scope.go:165: test logs captured to: outputs.zip/logTestInMemoryLock3786378506
    test_log_scope.go:76: use -show-logs to present logs inline
=== RUN   TestInMemoryLock/shared_lock_does_not_wait_for_shared_lock
POTENTIAL DEADLOCK: Recursive locking:
current goroutine 1079 lock 0xc00213d140
pkg/sql/vecindex/vecstore/in_memory_lock.go:62 vecstore.(*inMemoryLock).AcquireShared ??? <<<<<
pkg/sql/vecindex/vecstore/in_memory_lock.go:61 vecstore.(*inMemoryLock).AcquireShared ???
GOROOT/src/sync/atomic/type.go:144 atomic.(*Uint64).Load ???

Previous place where the lock was grabbed (same goroutine)
pkg/sql/vecindex/vecstore/in_memory_lock.go:62 vecstore.(*inMemoryLock).AcquireShared ??? <<<<<
pkg/sql/vecindex/vecstore/in_memory_lock.go:61 vecstore.(*inMemoryLock).AcquireShared ???
pkg/sql/vecindex/vecstore/in_memory_store_test.go:508 vecstore.TestInMemoryLock.func2 ???

=== RUN   TestInMemoryLock/lock_reentrancy

Parameters:

  • attempt=1
  • deadlock=true
  • run=1
  • shard=1
Help

See also: How To Investigate a Go Test Failure (internal)

This test on roachdash | Improve this report!

@cockroach-teamcity
Copy link
Member Author

sql/vecindex/vecstore.TestInMemoryLock failed on master @ 49cff91f3501494deaf038671bc643c194a0e3ca:

=== RUN   TestInMemoryLock
    test_log_scope.go:165: test logs captured to: outputs.zip/logTestInMemoryLock3268818913
    test_log_scope.go:76: use -show-logs to present logs inline
=== RUN   TestInMemoryLock/lock_reentrancy
=== RUN   TestInMemoryLock/shared_lock_does_not_wait_for_shared_lock
POTENTIAL DEADLOCK: Recursive locking:
current goroutine 1080 lock 0xc0021231a0
pkg/sql/vecindex/vecstore/in_memory_lock.go:62 vecstore.(*inMemoryLock).AcquireShared ??? <<<<<
pkg/sql/vecindex/vecstore/in_memory_lock.go:61 vecstore.(*inMemoryLock).AcquireShared ???
GOROOT/src/sync/atomic/type.go:144 atomic.(*Uint64).Load ???

Previous place where the lock was grabbed (same goroutine)
pkg/sql/vecindex/vecstore/in_memory_lock.go:62 vecstore.(*inMemoryLock).AcquireShared ??? <<<<<
pkg/sql/vecindex/vecstore/in_memory_lock.go:61 vecstore.(*inMemoryLock).AcquireShared ???
pkg/sql/vecindex/vecstore/in_memory_store_test.go:508 vecstore.TestInMemoryLock.func2 ???

Parameters:

  • attempt=1
  • deadlock=true
  • run=1
  • shard=1
Help

See also: How To Investigate a Go Test Failure (internal)

This test on roachdash | Improve this report!

@cockroach-teamcity
Copy link
Member Author

sql/vecindex/vecstore.TestInMemoryLock failed on master @ fe048f6e72edca59b4367463db1d0d00783f08ab:

=== RUN   TestInMemoryLock
    test_log_scope.go:165: test logs captured to: outputs.zip/logTestInMemoryLock2973449425
    test_log_scope.go:76: use -show-logs to present logs inline
=== RUN   TestInMemoryLock/lock_reentrancy
=== RUN   TestInMemoryLock/shared_lock_does_not_wait_for_shared_lock
POTENTIAL DEADLOCK: Recursive locking:
current goroutine 1101 lock 0xc00027b650
pkg/sql/vecindex/vecstore/in_memory_lock.go:62 vecstore.(*inMemoryLock).AcquireShared ??? <<<<<
pkg/sql/vecindex/vecstore/in_memory_lock.go:61 vecstore.(*inMemoryLock).AcquireShared ???
GOROOT/src/sync/atomic/type.go:144 atomic.(*Uint64).Load ???

Previous place where the lock was grabbed (same goroutine)
pkg/sql/vecindex/vecstore/in_memory_lock.go:62 vecstore.(*inMemoryLock).AcquireShared ??? <<<<<
pkg/sql/vecindex/vecstore/in_memory_lock.go:61 vecstore.(*inMemoryLock).AcquireShared ???
pkg/sql/vecindex/vecstore/in_memory_store_test.go:508 vecstore.TestInMemoryLock.func2 ???

Parameters:

  • attempt=1
  • deadlock=true
  • run=1
  • shard=1
Help

See also: How To Investigate a Go Test Failure (internal)

This test on roachdash | Improve this report!

@cockroach-teamcity
Copy link
Member Author

sql/vecindex/vecstore.TestInMemoryLock failed on master @ 9354770c7c6eb5a89437068d8c6a4accf8031b67:

=== RUN   TestInMemoryLock
    test_log_scope.go:165: test logs captured to: outputs.zip/logTestInMemoryLock1180759591
    test_log_scope.go:76: use -show-logs to present logs inline
=== RUN   TestInMemoryLock/lock_reentrancy
=== RUN   TestInMemoryLock/shared_lock_does_not_wait_for_shared_lock

Parameters:

  • attempt=1
  • deadlock=true
  • run=1
  • shard=1
Help

See also: How To Investigate a Go Test Failure (internal)

This test on roachdash | Improve this report!

@cockroach-teamcity
Copy link
Member Author

sql/vecindex/vecstore.TestInMemoryLock failed on master @ a653b4e4e6483cec7d65808ba4d55d8c63747a6e:

=== RUN   TestInMemoryLock
    test_log_scope.go:165: test logs captured to: outputs.zip/logTestInMemoryLock3303864706
    test_log_scope.go:76: use -show-logs to present logs inline
=== RUN   TestInMemoryLock/shared_lock_does_not_wait_for_shared_lock
POTENTIAL DEADLOCK: Recursive locking:
current goroutine 1101 lock 0xc0021312c0
pkg/sql/vecindex/vecstore/in_memory_lock.go:62 vecstore.(*inMemoryLock).AcquireShared ??? <<<<<
pkg/sql/vecindex/vecstore/in_memory_lock.go:61 vecstore.(*inMemoryLock).AcquireShared ???
GOROOT/src/sync/atomic/type.go:144 atomic.(*Uint64).Load ???

Previous place where the lock was grabbed (same goroutine)
pkg/sql/vecindex/vecstore/in_memory_lock.go:62 vecstore.(*inMemoryLock).AcquireShared ??? <<<<<
pkg/sql/vecindex/vecstore/in_memory_lock.go:61 vecstore.(*inMemoryLock).AcquireShared ???
pkg/sql/vecindex/vecstore/in_memory_store_test.go:508 vecstore.TestInMemoryLock.func2 ???

=== RUN   TestInMemoryLock/lock_reentrancy

Parameters:

  • attempt=1
  • deadlock=true
  • run=1
  • shard=1
Help

See also: How To Investigate a Go Test Failure (internal)

This test on roachdash | Improve this report!

craig bot pushed a commit that referenced this issue Dec 17, 2024
137105: sql/parser: Add new SQL grammar for row-level security (RLS) r=spilchen a=spilchen

This change introduces new SQL grammar to support row-level security (RLS). All of the statements, with the exception of SHOW POLICIES match the postgres grammar. The added statements and options are as follows:
```
DROP POLICY [ IF EXISTS ] name ON table_name [ CASCADE | RESTRICT ]

CREATE POLICY name ON table_name

[ AS { PERMISSIVE | RESTRICTIVE } ]
[ FOR { ALL | SELECT | INSERT | UPDATE | DELETE } ]
[ TO { role_name | PUBLIC | CURRENT_USER | SESSION_USER } [, ...] ]
[ USING ( using_expression ) ]
[ WITH CHECK ( check_expression ) ]

ALTER POLICY name ON table_name RENAME TO new_name

ALTER POLICY name ON table_name

[ TO { role_name | PUBLIC | CURRENT_USER | SESSION_USER } [, ...] ]
[ USING ( using_expression ) ]
[ WITH CHECK ( check_expression ) ]

ALTER TABLE … DISABLE ROW LEVEL SECURITY
ALTER TABLE … ENABLE ROW LEVEL SECURITY
ALTER TABLE … FORCE ROW LEVEL SECURITY
ALTER TABLE … NO FORCE ROW LEVEL SECURITY

CREATE ROLE name [[ WITH ] [BYPASSRLS | NOBYPASSRLS]]
ALTER ROLE role_specification [ WITH ] [BYPASSRLS | NOBYPASSRLS]

SHOW POLICIES FOR <table_name>
```

Currently, attempting to use any of the new statements or options will return an unimplemented error. Documentation for this grammar is temporarily omitted and will be added closer to the feature's release, expected in version 25.2.

Epic: CRDB-11724
Closes: #136692
Release note: None

137363: teamcity: add script to run PGO build and collect profiles r=rail a=rickystewart

This script ties together the pieces of the workflow: start a TC job, wait for it to finish, downloads the artifacts, parses the profiles, merges them, and produces a final result.

Part of: CRDB-44692
Epic: CRDB-41952
Release note: None

137374: vecstore: disable deadlock linting for inMemoryLock r=drewkimball a=andy-kimball

Do not use syncutil.RWMutex in the inMemoryLock class, because deadlock detection reports spurious failures. Different partitions in the vector index can be locked in different orders by merge, split, format and other operations. In all these cases, we first acquire the in-memory store's structure lock to prevent deadlocks. But the deadlock detection package is not smart enough to realize this and reports false positives.

Epic: CRDB-42943
Fixes: #136958.
Fixes: #136960.

Release note: None

137448: sql/types, sql/stats: fix SQLStringFullyQualified for arrays of UDTs r=rytaft a=michae2

Starting in v24.2 we added `SQLStringFullyQualified`, but it looks like it didn't work for arrays of user-defined types. This is only used in a few places, including the output of `SHOW STATISTICS USING JSON`.

This commit:
1. fixes `SQLStringFullyQualified` for arrays of UDTs
2. adds the `t.TypeMeta.Name == nil` guard to `SQLString` for composite UDTs
3. sets a formatting flag in `stats.(*JSONStatistic).SetHistogram` so that we get fully-qualified type annotations on histogram upper bounds

No release note because the output of `SHOW STATISTICS USING JSON` isn't documented.

Fixes: #137443

Release note: None

137568: distsql: eliminate ctx allocation in setupFlow r=mgartner a=mgartner

The `ctx` parameter of `setupFlow` is no longer captured by closures,
preventing it from being heap allocated.

Epic: None

Release note: None


137613: orchestration: released CockroachDB version 24.3.1. Next version: 24.3.2 r=rail a=cockroach-teamcity


Release note: None
Epic: None
Release justification: non-production (release infra) change.


Co-authored-by: Matt Spilchen <[email protected]>
Co-authored-by: Ricky Stewart <[email protected]>
Co-authored-by: Andrew Kimball <[email protected]>
Co-authored-by: Michael Erickson <[email protected]>
Co-authored-by: Marcus Gartner <[email protected]>
Co-authored-by: Justin Beaver <[email protected]>
craig bot pushed a commit that referenced this issue Dec 17, 2024
137374: vecstore: disable deadlock linting for inMemoryLock r=drewkimball a=andy-kimball

Do not use syncutil.RWMutex in the inMemoryLock class, because deadlock detection reports spurious failures. Different partitions in the vector index can be locked in different orders by merge, split, format and other operations. In all these cases, we first acquire the in-memory store's structure lock to prevent deadlocks. But the deadlock detection package is not smart enough to realize this and reports false positives.

Epic: CRDB-42943
Fixes: #136958.
Fixes: #136960.

Release note: None

137448: sql/types, sql/stats: fix SQLStringFullyQualified for arrays of UDTs r=rytaft a=michae2

Starting in v24.2 we added `SQLStringFullyQualified`, but it looks like it didn't work for arrays of user-defined types. This is only used in a few places, including the output of `SHOW STATISTICS USING JSON`.

This commit:
1. fixes `SQLStringFullyQualified` for arrays of UDTs
2. adds the `t.TypeMeta.Name == nil` guard to `SQLString` for composite UDTs
3. sets a formatting flag in `stats.(*JSONStatistic).SetHistogram` so that we get fully-qualified type annotations on histogram upper bounds

No release note because the output of `SHOW STATISTICS USING JSON` isn't documented.

Fixes: #137443

Release note: None

137568: distsql: eliminate ctx allocation in setupFlow r=mgartner a=mgartner

The `ctx` parameter of `setupFlow` is no longer captured by closures,
preventing it from being heap allocated.

Epic: None

Release note: None


Co-authored-by: Andrew Kimball <[email protected]>
Co-authored-by: Michael Erickson <[email protected]>
Co-authored-by: Marcus Gartner <[email protected]>
@craig craig bot closed this as completed in 1203a72 Dec 17, 2024
@craig craig bot closed this as completed in #137374 Dec 17, 2024
@github-project-automation github-project-automation bot moved this from Triage to Done in SQL Queries Dec 17, 2024
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. release-blocker Indicates a release-blocker. Use with branch-release-2x.x label to denote which branch is blocked. T-sql-queries SQL Queries Team
Projects
Archived in project
Development

Successfully merging a pull request may close this issue.

1 participant