membuffer: fix data race when `IsReadOnly` and Set may be concurrently invoked

[you06]

ref pingcap/tidb#56178

TiDB concurrently call IsReadOnly and some write operations in union statement, so we need to avoid data race by mutex.

[ti-chi-bot[bot]]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: cfzjywxk

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

• ~~OWNERS~~ [cfzjywxk]

Approvers can indicate their approval by writing /approve in a comment Approvers can cancel approval by writing /approve cancel in a comment

[ti-chi-bot[bot]]

[LGTM Timeline notifier]

Timeline:

• 2024-10-18 08:12:09.98758155 +0000 UTC m=+604327.136491367: :ballot_box_with_check: agreed by cfzjywxk.

[ekexium]

I understand it fixes the data race. I'm just wondering whether it means we are using it correctly. What is the expected behavior when dirty is accessed concurrently?

I haven't checked the case yet but a first rough thought might be like:

Thread-1 gets dirty = false
Thread-2 writes dirty = true
Thread-1 does something that depends on dirty = false? // this can be problematic?

[you06]

Is there a test case in tidb to verify the fix?

There is a test case in the issue.

[you06]

I understand it fixes the data race. I'm just wondering whether it means we are using it correctly. What is the expected behavior when dirty is accessed concurrently?

I haven't checked the case yet but a first rough thought might be like:

Thread-1 gets dirty = false
Thread-2 writes dirty = true
Thread-1 does something that depends on dirty = false? // this can be problematic?

For this question, we're just avoiding the race between UpdateFlags and IsDirty, if IsDirty() == true, the result from membuffer will be not-exist error and it finally read from the store.

This is a large topic because TiDB used to pretend it's executor is running in serial mode. So there may be some hidden issues with high possibility.