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/multitenancy: unexpected concurrency for flow error when creating stats #50049

Closed
Tracked by #52617
asubiotto opened this issue Jun 10, 2020 · 1 comment · Fixed by #52621
Closed
Tracked by #52617

sql/multitenancy: unexpected concurrency for flow error when creating stats #50049

asubiotto opened this issue Jun 10, 2020 · 1 comment · Fixed by #52621
Assignees
Labels
A-multitenancy Related to multi-tenancy C-bug Code not up to spec/doc, specs & docs deemed correct. Solution expected to change code/behavior.

Comments

@asubiotto
Copy link
Contributor

Seen in enums logic test when run with 3node-tenant config:

CREATE STATISTICS s FROM greeting_stats;
    -- FAIL
    TestLogic/3node-tenant/enums: logic.go:2251:

        testdata/logic_test/enums:368:
        expected success, but found
        (XX000) internal error: unexpected concurrency for a flow that was forced to be planned locally
        distsql_running.go:403: in Run()
        DETAIL: stack trace:
        github.com/cockroachdb/cockroach/pkg/sql/distsql_running.go:403: Run()
        github.com/cockroachdb/cockroach/pkg/sql/distsql_plan_stats.go:257: planAndRunCreateStats()
        github.com/cockroachdb/cockroach/pkg/sql/create_stats.go:427: func2()
        github.com/cockroachdb/cockroach/pkg/kv/db.go:707: func1()
        github.com/cockroachdb/cockroach/pkg/kv/txn.go:800: exec()
        github.com/cockroachdb/cockroach/pkg/kv/db.go:706: Txn()
        github.com/cockroachdb/cockroach/pkg/sql/create_stats.go:414: Resume()
        github.com/cockroachdb/cockroach/pkg/jobs/registry.go:821: stepThroughStateMachine()
        github.com/cockroachdb/cockroach/pkg/jobs/registry.go:952: func1()
        github.com/cockroachdb/cockroach/pkg/util/stop/stopper.go:323: func1()
        runtime/asm_amd64.s:1373: goexit()

We probably don't check planCtx.isLocal (which is forced in a tenant) somewhere we should be.

@asubiotto asubiotto added C-bug Code not up to spec/doc, specs & docs deemed correct. Solution expected to change code/behavior. A-multitenancy Related to multi-tenancy A-multitenancy-blocker labels Jun 10, 2020
@asubiotto
Copy link
Contributor Author

This happens because we force local planning of all flows as a tenant. Stats collection processors do not implement RowSource (they only implement Run) and so cannot be "fused" together. This inability to fuse the processors is what causes this error.

The error's goal is to protect against concurrent usage of the RootTxn which wouldn't happen in this case. Here two solutions:

  1. Make this check more lenient. We could whitelist processors by having them implement SafeForConcurrentLocalExecution. This'll be the easiest cc @andreimatei since you introduced this check.
  2. Implement these processors as RowSources so that they can be fused. Was there any reason this wasn't done before? cc @rytaft

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-multitenancy Related to multi-tenancy C-bug Code not up to spec/doc, specs & docs deemed correct. Solution expected to change code/behavior.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants