Read TiDB source code with questions: Hive metadata uses TiDB startup to report errors

"Reading Source Code with Questions Series"-Opening

Partners who have been active in the TiDB community for a long time should know that in the past, we had the "TiDB Source Code Reading Series" called the 24 chapters, and the "TiKV Source Code Analysis Series" for TiKV and the "Deep Dive TiKV Series of Articles". The content of these series of articles is very in-depth and can help you understand the implementation and basic principles of TiDB and TiKV from very detailed principles.

However, many partners who are active in the TiDB community need to read more simple and more related source code articles about the problems they encounter when using TiDB in their daily work. This article is the first attempt of the "Reading Source Code with Questions" series. In the process of locating and solving a simple problem encountered by users, the relevant code is introduced together. I hope to help you learn the source code of TiDB and TiKV from a different perspective and with different problem granularity.

AskTUG has feedback on the problems encountered by many users in their daily use of TiDB, and these problems can become source code analysis materials similar to this article. If this article can create value for everyone, then we must strive to continue to build the "Reading Source Code Series with Questions" into a source code reading series that is as popular as our predecessors.

problem

Recently, I received feedback from users on the AskTUG forum that setting the SERIALIZABLE transaction isolation level failed when using TiDB as the Hive metastore database. Moreover, the user still cannot solve the problem as expected after performing the SET GLOBAL tidb_skip_isolation_level_check=1 operation according to the document's suggestion. Considering that Zhihu was officially launched a year ago and has been using the 4.0.x series of TiDB as the Hive metastore database, and users still cannot successfully deploy Hive metastore on TiDB according to the instructions, which means that TiDB is likely to be different An incompatible behavior change occurred between the versions of. Next, let us start with troubleshooting and learn to understand the source code behind the corresponding functions.

Verification process

With the help of tiup, we can easily start multiple different versions of TiDB to test and verify the behavior of the transaction isolation level.

First, let's start the TiDB cluster of version 5.0.0 to prepare for testing

Next, we use the connection command prompted by tiup to connect to the test cluster using mysql client. After setting SET GLOBAL tidb_skip_isolation_level_check=1, use SET TRANSACTION ISOLATION LEVEL SERIALIZABLE to verify that the behavior is as expected. It shows that the behavior of TiDB 5.0 series is consistent with 4.0, which can support the operation of Hive metastore.

Next we start the TiDB cluster of version 5.1.0 to prepare for testing

Similarly, we use mysql client to connect to the test cluster. After setting SET GLOBAL tidb_skip_isolation_level_check=1 and rebuilding the link to ensure that the settings take effect, use SET TRANSACTION ISOLATION LEVEL SERIALIZABLE to still receive error reports. It indicates that the behavior from the TiDB 5.1 series is inconsistent with previous versions and cannot meet the requirements of the Hive metastore.

problem analysis

First, we need to checkout a copy of the latest TiDB code (git hash: 649ed6abc9790cfdd2a17065118379d8abcc7595) to view the logic related to transaction isolation level verification. In order to quickly locate the code where the relevant logic is located, we can perform a text search on the string SERIALIZABLE in the root directory of the TiDB code, and quickly locate the code files that may be related to this.

We found that there are actually two files containing the string SERIALIZABLE, and the only file that determines the isolation level for processing is sessionctx/variable/varsutil.go. After opening the file, we found that this is the logic to judge the isolation level and decide whether to pass according to the tidb_skip_isolation_level_check setting.

We can compare the TiDB code of version 5.0 (git hash: 53251a9731da02ad9ee5abed9f27a14c7dea33a4) that meets expectations to quickly locate the difference in behavior between the two caused by those changes. Similarly, we quickly locate sessionctx/variable/sysvar.go and sessionctx/variable/session.go through string matching, both of which have conditional processing on the isolation level.

These two different check logics are very similar, they both try to get the setting of the TiDBSkipIsolationLevelCheck variable, and decide whether to release it according to the set value. When we compare the logic here with the logic in the master code, we find that their essential difference is very small. In 5.0, a built-in tool function GetSessionSystemVar is used to obtain the variable value, and the master code directly accesses the SessionVars systems variable table to obtain the current value of the TiDBSkipIsolationLevelCheck variable. Looking further at the implementation of GetSessionSystemVar in 5.0, we find that this tool function is responsible for further searching in the global variable table when the session variable is not set, and placing the found result in the systems variable table of SessionVars for subsequent searching.

According to the current clues, in 5.1 a certain code refactoring tried to merge two similar repeated isolation level check logics into a common logic, bypassing the tool function and directly accessing the systems variable table. This way of accessing the variable table does not have the ability of the previous tool function to automatically roll back the global variable settings. Knowing that the repair is very simple, just use the similar tool function GetSessionOrGlobalSystemVar in the current TiDB to read the variable value of TiDBSkipIsolationLevelCheck to restore the expected behavior.

After repairing and completing the build, test TiDB's behavior again as expected.

Submit repair

According to the code contribution process of the TiDB community standard, we first create a new issue to clearly describe the problems found, the recurrence method, and the expected behavior.

After creating the issue, we can submit the repair logic to our fork warehouse and create a PR. During the creation process, we need to fill in the PR information template according to the actual situation.

After the creation is completed, the CI system will conduct a series of responsible inspections and perform necessary tests on the submitted PR, in addition to the automated verification of these systems. Other community contributors will code review the PR. After enough contributors from TiDB Reviewer and above have praised the PR, the changes can be merged into the main project.

Soon after the PR submission, I got the review feedback from @morgo. The feedback pointedly pointed out that the real reason behind the problem was the incorrect change in the initialization behavior of the TiDBSkipIsolationLevelCheck variable in PR #24836. The skipInit: true initialization field in the definition of the TiDBSkipIsolationLevelCheck variable was removed. It can ensure that the global variable values are correctly copied to the session when the session is initialized, and the logic behavior of the previous isolation level check can be restored to normal. According to this clue, the code is modified and the actual test proves that the performance is in line with expectations. Next, let us continue to analyze the source code related to skipInit to find out.

All reading operations of skipInit variables in the code are encapsulated in the SkipInit function in the figure above. From the figure below, we can see that the SkipInit method is used to skip some variables in the process of initializing a new session variable cache.

Next, newSessionCache is updated to the session variable and provides external access through the GetSessionCache method in the figure below.

The GetSessionCache method has only one caller, loadCommonGlobalVariablesIfNeeded, so the impact of skipInit on the initialization process of system variables is very clear.

When the session is created, the variables not marked as skipInit will be updated to the session variable table in the form of the initial value of the variable, which is the aforementioned systems variable table. When we restore skipInit of TiDBSkipIsolationLevelCheck to false, the global variable tidb_skip_isolation_level_check can be correctly copied to the user session during this initialization process, so that the behavior of adjusting the session transaction isolation level is in line with user expectations.

After the problem is resolved, you may also ask under what circumstances skipInit needs to be set to true. In the PR #24836 that introduced this feature, we can know that some variables that are not suitable for copying into the session during the initialization process will use this tag to achieve the blacklist function. During this refactoring process, TiDBSkipIsolationLevelCheck was incorrectly set in the blacklist, which caused an abnormal behavior in the 5.1 starting version.

What problem does this PR solve?

Problem Summary:Currently the builtinGlobalVariable feature is a source of bugs because even though a sysvar is added, it is not automatically copied to new sessions. This behavior is also not MySQL compatible, where it is expected a sysvar of session scope should be copied on session init.Fixing the full incompatibility is a little bit more complicated, but this takes the initial step of inverting from an allow list to a deny list, but is otherwise functionally compatible.This also includes the fix from #24835 should this PR supercede it.

What is changed and how it works?

What's Changed:A variable on the SysVar struct can now be set to skipInit. By default it will not skip for session-scope variables, which is why it is now a deny list.However, it will always skip for noop variables, which helps keep the memory footprint of new sessions slightly lower.

Related changes

None
There will need to be followup PRs to handle the specific skipInit variables; some probably don't need to be on this list. The global-only variables that are hard-coded into the SkipInit function will also need removing.
postscript

Thanks to the enthusiastic users who reported the abnormal behavior of TiDB to the community. I am very sorry that we could not locate and solve the problem in the first time when the failure occurred. But we still hope that after the new version is released to fix this problem, TiDB can play a positive role in supporting Hive metastore and even more business scenarios for you.