Add Flush and BufferGet requests for pipelined DML

[ekexium]

Ref tikv/tikv#16291
Don't merge it until corresponding work finishes.

[you06]

Need to rebase master, I cannot build the latest TiDB with this PR because some protocols are missing.

[you06]

rest LGTM

[you06]

Shall we add a variable to control whether enable snapshot read after buffer read miss? This may save one RPC.

[cfzjywxk]

A similar question, if it's designed to support both buffer-read and snapshot read, or do we need another interface to implement "UnionStoreBatchGet" at tikv side?

[ekexium]

Shall we add a variable to control whether enable snapshot read after buffer read miss? This may save one RPC.

We can do it later when we are about to implement this. We don't have to finalize all details in one PR.

A similar question, if it's designed to support both buffer-read and snapshot read, or do we need another interface to implement "UnionStoreBatchGet" at tikv side?

BufferBatchGet only serves buffer read. Its semantics is "reading the content offloaded to TiKV in the membuffer"
Snapshot read does not change.
Something like UnionStoreBatchGet is not needed for now(before we implement the optimization to save one RPC), as unionstore.get is simply implemented as "buffer.get, if not found, try snapshot.get". This implementation does not have to change at all.
The only thing we need to change is how we do buffer read. Does that sound straightforward and reasonable?

Previous:
membuffer.get = memdb.get
snapshot.get = tikv.get
unionstore.get = membuffer.get +? snapshot.get = memdb.get +? tikv.get

Now:
membuffer.get = mutable_memdb.get +? immutable_memdb.get +? tikv.buffer_batch_get
snapshot.get = tikv.get
unionstore.get = membuffer.get +? snapshot.get = mutable_memdb.get +? immutable_memdb.get +? tikv.buffer_batch_get +? tikv.get

[cfzjywxk]

Another way for reading a transaction's own uncommitted data in tikv:

• kv-client: For pipelined-dml read, adding a flag in the existing BatchGet request, it indicates whether the current read is membuf read in tikv or union store read in tikv.

• tikv: reuse the same BatchGet interface.

  • storage: implement the buffer_batch_get and union_store_batch_get methods.

So we could avoid introducing the BufferGet interface and do union store get within one RPC request.

[ekexium]

Another way for reading a transaction's own uncommitted data in tikv:

• kv-client: For pipelined-dml read, adding a flag in the existing BatchGet request, it indicates whether the current read is membuf read in tikv or union store read in tikv.

• tikv: reuse the same BatchGet interface.

  • storage: implement the buffer_batch_get and union_store_batch_get methods.

So we could avoid introducing the BufferGet interface and do union store get within one RPC request.

So the only difference is in the grpc level, whether to use 3 different RPCs for snapshot read, buffer read, and union read; or use one RPC for all of them and use flags to differentiate.

Seems a trade-off between interface simplicity and clarity. Imagine when we see a batch_get from logs or metrics, we would have no idea what it is reading.

[cfzjywxk]

We essentially need to support one RPC to complete the membuf read + snapshot read, regardless of whether or not a separate KV interface is split out. This scalability needs to be reserved.
It doesn't seem to introduce too much protocol complexity to separately split out BufferBatchGetRequest, although it introduces a new interface. However, I don't think we need to introduce another UnionStoreBatchGetRequest interface.

[cfzjywxk]

@MyonKeminta PTAL

[MyonKeminta]

It doesn't seem to introduce too much protocol complexity to separately split out BufferBatchGetRequest, although it introduces a new interface. However, I don't think we need to introduce another UnionStoreBatchGetRequest interface.

I agree with this. We can regard it as an RPC interface that supports reading with the buffer, including buffer reading only and mixed (union) reading.

But my problem is whether the word "buffer" is proper here...

[ekexium]

@MyonKeminta any suggestions on names?

[MyonKeminta]

How about using the word "staged"?
..And maybe i'd better approve this PR for now as I don't have any better idea...

[cfzjywxk]

LGTM.