Skip to content

nightly: Random Syntax Tests often times out #146320

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

Closed
yuzefovich opened this issue May 7, 2025 · 0 comments · Fixed by #146328
Closed

nightly: Random Syntax Tests often times out #146320

yuzefovich opened this issue May 7, 2025 · 0 comments · Fixed by #146328
Assignees
Labels
branch-master Failures and bugs on the master branch. branch-release-25.1 branch-release-25.2 C-bug Code not up to spec/doc, specs & docs deemed correct. Solution expected to change code/behavior. target-release-25.1.8 target-release-25.2.2 target-release-25.3.0

Comments

@yuzefovich
Copy link
Member

yuzefovich commented May 7, 2025

On a quick glance it's because crdb_internal.fingerprint doesn't respect the stopper's shutdown signal.

Jira issue: CRDB-50472

@yuzefovich yuzefovich added C-bug Code not up to spec/doc, specs & docs deemed correct. Solution expected to change code/behavior. branch-master Failures and bugs on the master branch. branch-release-25.2 branch-release-25.1 labels May 7, 2025
@yuzefovich yuzefovich self-assigned this May 7, 2025
craig bot pushed a commit that referenced this issue May 8, 2025
145435: sql: add sqlcommenter support r=kyle-a-wong a=kyle-a-wong

commit 1/2:

sql: refactor parser.go to take parser options
Refactored parser functions to use a new ParserOptions type configure various options when parsing sql. There are now two types of Parse functions, Parse*() and Parse*WithOptions().  Parse* will use default parse options and the WithOptions alternative allows specfic options to be provided.

Epic: None
Release note: None

----

commit 2/2:

sql: retain SQL commenter tags
SQL commenter library is used by ORM libraries and customer applications to add application context to the queries using inline SQL comments.

This PR includes the following changes:
- A new cluster setting, `sql.sqlcommenter.enabled`, which retains comments during parsing phase.
- If the comments follow [SQL commenter specification](https://google.github.io/sqlcommenter/spec/#introduction), they are converted to SQL tags.
- Changes to insights stats to also include SQL commenter tags.
- Changes to the crdb_internal.*_execution_insights virtual tables to have   a new `query_tags` column.
- Addition of log tags on the conn_ex context. Each sql commenter tag will be added as a log tag, prefixed with 'querytag-'.

Epic: None
Release note (sql change): Added support for query tagging, which allows users to add query tags to their sql statements via comments. These query tags will be logged as log tags and will be included in all log entries logged during the executing of a sql statement. These tags will also be included in traces. Finally, these tags will be included in the `cluster_execution_insights` and `node_exeuction_insights` virtual tables in a new `query_tags` JSONB column. This feature can be enabled with the `sql.sqlcommenter.enabled` cluster setting and is disabled by default. Comments must adhere to the sqlcommenter spec, as defined here: https://google.github.io/sqlcommenter/spec/


146120: kvcoord: fix the write buffer's adjustError r=yuzefovich,stevendanna a=miraradeva

Previously, `adjustError` handled the cases of stripped requests (not sent to KV at all) and non-transformed requests (sent directly to KV), but didn't always handle the cases of transformed requests correctly.

This commit simplifies the logic in `adjustError` and adds additional testing.

Fixes: [#144694](#144694)

Release note: None

146285: roachtest: init providers in main(), not init() r=tbg a=tbg

Initializing them in an init function means we're calling this method in tests,
where it is likely not useful and may even be implicated in opaque test
timeouts such as [1].

[1]: https://cockroachlabs.slack.com/archives/CJ0H8Q97C/p1746629472748029?thread_ts=1746626113.617579&cid=CJ0H8Q97C

The roachtest job still works[^1]:

<img width="1352" alt="image" src="https://github.com/user-attachments/assets/9233459b-5fd6-4e91-a8a5-deba1177eaab" />

[^1]: https://teamcity.cockroachdb.com/buildConfiguration/Cockroach_Nightlies_RoachtestNightlyGceBazel/19646638?buildTab=overview&hideProblemsFromDependencies=false&hideTestsFromDependencies=false&expandBuildChangesSection=true

Epic: none


146298: revert "sql: remove old hyperloglog version" r=yuzefovich a=yuzefovich

This reverts commits 088306f and  70e9742. We haven't yet advanced necessary versions on master to make an incompatible change with 24.3 without any fallout, so let's revert the change for now (a couple of weeks).

Fixes: #146268.

146318: roachtest: update backup fixtures collection URIs to millisecond precision r=msbutler a=kev-cao

The current backup fixture driver creates backups to collection URIs that are formatted based on the current time, accurate to the minute. This causes test flakes when two roachtest fixture runs run at the same minute and create backup schedules that backup to the same collection URI. To avoid this, we update the fixtures to be accurate to the millisecond.

Fixes: #146031, #146030

Release note: None

146328: sql: fix crdb_internal.fingerprint in edge cases r=yuzefovich a=yuzefovich

This commit fixes a couple of bugs that were introduced in 5e42a93 in `crdb_internal.fingerprint` builtin. Namely, after that change it was possible that the new coordinator goroutine never exits (because we forgot to check for the context cancellation) which could block the server shutdown. It could also happen if we had a `statement_timeout` set.

Relatedly, if the coordinator encounters an error itself (rather than receives it from one of the workers), the coordinator would exit without notifying the workers. This was the case since we only close the work channel after having pushed out all sub-spans. I don't think this was problematic in practice, but this is now fixed by explicitly canceling the context of workers.

Fixes: #146320.

Release note: None

Co-authored-by: Chandra Thumuluru <[email protected]>
Co-authored-by: Mira Radeva <[email protected]>
Co-authored-by: Tobias Grieger <[email protected]>
Co-authored-by: Yahor Yuzefovich <[email protected]>
Co-authored-by: Kevin Cao <[email protected]>
@craig craig bot closed this as completed in #146328 May 8, 2025
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. branch-release-25.1 branch-release-25.2 C-bug Code not up to spec/doc, specs & docs deemed correct. Solution expected to change code/behavior. target-release-25.1.8 target-release-25.2.2 target-release-25.3.0
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant