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-queries: TestLogic_lookup_join_local flaked on master #95799

Closed
msbutler opened this issue Jan 24, 2023 · 3 comments · Fixed by #95838
Closed

sql-queries: TestLogic_lookup_join_local flaked on master #95799

msbutler opened this issue Jan 24, 2023 · 3 comments · Fixed by #95838
Assignees
Labels
C-test-failure Broken test (automatically or manually discovered). T-sql-queries SQL Queries Team

Comments

@msbutler
Copy link
Collaborator

msbutler commented Jan 24, 2023

See CI failure here

Jira issue: CRDB-23746

@msbutler msbutler added C-test-failure Broken test (automatically or manually discovered). T-sql-queries SQL Queries Team labels Jan 24, 2023
@yuzefovich
Copy link
Member

Seems like lookup_join_local gets extremely slow, reproducible with ./dev testlogic base -v --stream-output --files=lookup_join_local --config=local -- --test_env=COCKROACH_RANDOM_SEED=-1150544666816831660. The stack trace shows the usage of the streamer, but even if I disable it, then the query is still slow. Probably it's one of the metamorphic variables that is slowing everything down significantly.

@yuzefovich yuzefovich self-assigned this Jan 25, 2023
@yuzefovich
Copy link
Member

Indeed, disabling the randomization on logictest-small-engine-blocks makes this test go fast.

@erikgrinaker how would you feel about this diff:

diff --git a/pkg/sql/logictest/logic.go b/pkg/sql/logictest/logic.go
index bdfcad17591..df7bc3b8ef6 100644
--- a/pkg/sql/logictest/logic.go
+++ b/pkg/sql/logictest/logic.go
@@ -1328,7 +1328,7 @@ func (t *logicTest) newCluster(
        shouldUseMVCCRangeTombstonesForPointDeletes := useMVCCRangeTombstonesForPointDeletes && !serverArgs.DisableUseMVCCRangeTombstonesForPointDeletes
        ignoreMVCCRangeTombstoneErrors := supportsMVCCRangeTombstones &&
                (globalMVCCRangeTombstone || shouldUseMVCCRangeTombstonesForPointDeletes)
-       useSmallEngineBlocks := smallEngineBlocks && !serverArgs.DisableSmallEngineBlocks
+       useSmallEngineBlocks := smallEngineBlocks && !serverArgs.DisableSmallEngineBlocks && !serverArgs.ForceProductionValues
 
        params := base.TestClusterArgs{
                ServerArgs: base.TestServerArgs{

Not sure if it's worth digging into this slowness in general, but this diff at least makes the slowness in this particular case go away.

@erikgrinaker
Copy link
Contributor

erikgrinaker commented Jan 25, 2023

Honestly, I'd be fine with just removing the smallEngineBlocks metamorphic variable entirely, given the amount of issues it's causing. It's useful to make sure time-bound iterators work properly, but we can probably have more targeted tests for it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-test-failure Broken test (automatically or manually discovered). T-sql-queries SQL Queries Team
Projects
Archived in project
Development

Successfully merging a pull request may close this issue.

3 participants