Optimize fetch_entries_to function to avoid stucking write too long.#382
Conversation
Signed-off-by: lucasliang <nkcs_lykx@hotmail.com>
|
Skipping CI for Draft Pull Request. |
Signed-off-by: lucasliang <nkcs_lykx@hotmail.com>
| for i in ents_idx.iter() { | ||
| vec.push(read_entry_from_file::<M, _>(self.pipe_log.as_ref(), i)?); | ||
| vec.push({ | ||
| match read_entry_from_file::<M, _>(self.pipe_log.as_ref(), i) { |
There was a problem hiding this comment.
Can we ensure the safety to read entry without holding the lock? Is it possible that some entries are truncated and target wal files are Gced(or reused) before this read, then the result of this read is undefined behavior
There was a problem hiding this comment.
Yep. It's ensured.
Is it possible that some entries are truncated and target wal files are Gced(or reused) before this read
-
File Access Atomicity in Raft-Engine
Inraft-engine, file access (.raftlog) is atomic. The GC operation acquires a mutex and deletes the entire file before allowing other accesses. If a read operation successfully returns the target bytes, the result is guaranteed valid. -
Index Consistency Handling
This PR further ensures consistency by returningError(e)if the Memtable index is updated (by either background rewrite or foreground write threads) during a read. Then it will automatically retry with the latest index to fetch the correct entry bytes.
There was a problem hiding this comment.
Index Consistency Handling
Say if an entry is rewritten and the old wal file is purged before the real access, seems we still can read it with the old entry info as the file handle is changed then seems we still returns an error even if the entay is actually vaild.
There was a problem hiding this comment.
If it uses a stale file handle to get the entry, it will get the Error(OutOfSeq), ref:
https://github.com/LykxSassinator/raft-engine/blob/392f5e66f8286dc1b6d7cf69f2bc20ed72d40123/src/file_pipe_log/pipe.rs#L238
So, if the first fetch uses a stale index to get the entry, it will return an Error and trigger the second retry, where it will use the latest index to access the entry.
Signed-off-by: lucasliang <nkcs_lykx@hotmail.com>
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: glorv, overvenus The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
…ng. (tikv#382) Signed-off-by: lucasliang <nkcs_lykx@hotmail.com>
…ng. (tikv#382) Signed-off-by: lucasliang <nkcs_lykx@hotmail.com>
…ng. (#382) Signed-off-by: lucasliang <nkcs_lykx@hotmail.com>
Background
PR #370 fixed a panic issue caused by concurrent updates to
Memtablefollowed by reads on stale indexes. However, we observed that this fix introduced performance regressions under concurrent read/write workloads.Issue
When
fetch_entries_toretrieves a large batch of entries from disk, write operations are blocked until the read completes, delayingMemtableupdates and degrading throughput.Solution
This PR optimizes
fetch_entries_toto reduce contention and improve performance under mixed workloads.Results