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

Remove "tikv" from isolation-read.engines in config file will make tidb unable to execute some DDL jobs as ddl worker #45202

Closed
time-and-fate opened this issue Jul 6, 2023 · 3 comments
Assignees
Labels
affects-5.4 This bug affects 5.4.x versions. affects-6.1 affects-6.5 affects-7.1 severity/moderate sig/sql-infra SIG: SQL Infra type/bug The issue is confirmed as a bug.

Comments

@time-and-fate
Copy link
Member

time-and-fate commented Jul 6, 2023

Bug Report

Please answer these questions before submitting your issue. Thanks!

1. Minimal reproduce step (Required)

tidb config file:

[isolation-read]
engines = ["tiflash", "tidb"]
use test
create table t(a int);
alter table t modify column a int not null;

2. What did you expect to see? (Required)

The DDL runs successfully.

3. What did you see instead (Required)

ERROR 1815 (HY000): Internal : No access path for table 't' is found with 'tidb_isolation_read_engines' = '', valid values can be 'tikv'.

In another case, if there are two tidbs, one tidb (tidb1) with config file above is the ddl owner, and the other (tidb2) uses the default config. If we send the DDL to tidb2, it will block for a long time and return error, and there will be error log the same as above in tidb1.

4. What is your TiDB version? (Required)

v5.0.0 - v7.2.0

@time-and-fate time-and-fate added the type/bug The issue is confirmed as a bug. label Jul 6, 2023
@time-and-fate
Copy link
Member Author

time-and-fate commented Jul 6, 2023

Some details in the current implementation:

  1. For a new session, the IsolationReadEngines are set according to the config file:

    for _, engine := range config.GetGlobalConfig().IsolationRead.Engines {
    switch engine {
    case kv.TiFlash.Name():
    vars.IsolationReadEngines[kv.TiFlash] = struct{}{}
    case kv.TiKV.Name():
    vars.IsolationReadEngines[kv.TiKV] = struct{}{}
    case kv.TiDB.Name():
    vars.IsolationReadEngines[kv.TiDB] = struct{}{}
    }
    }

  2. The logic of "isolation read engines" has a special check for mysql database. Tables in mysql will not be affected by the setting:

    if dbName.L == mysql.SystemDB {
    return paths, nil
    }

    So, many other internal sessions are not affected by this issue, because they only need to read from the mysql database. And this also means "isolation read engines" are ineffective on mysql database.

  3. For some internal sessions (including this case), they are set not to load the newest global variables:

    se.sessionVars.CommonGlobalLoaded = true

    This doesn't cause this bug itself, but prevents us from workaround by updating the global variables.

  4. For some DDL jobs, the DDL worker needs to read from the specified table of the DDL (like table t in this case):
    doModifyColumn() -> modifyColsFromNull2NotNull() -> checkForNullValue()

@time-and-fate
Copy link
Member Author

I think the internal session should not be affected by the setting of "isolation read engines" to make sure all internal SQLs can succeed as expected.

@bb7133
Copy link
Member

bb7133 commented Jan 9, 2024

IMO for now the current behavior is expected: if "tikv" is removed from `isolation-read.engines' TiDB does not guarantee the interactions with TiKV(like executing DDL) work fine.

If we think this is a problem, it can be treated as a further enhancements in the future.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
affects-5.4 This bug affects 5.4.x versions. affects-6.1 affects-6.5 affects-7.1 severity/moderate sig/sql-infra SIG: SQL Infra type/bug The issue is confirmed as a bug.
Projects
None yet
Development

No branches or pull requests

4 participants