search

tikv / raft-engine

A persistent storage engine for Multi-Raft log
Apache License 2.0
565 stars 88 forks source link

Change `version` format to u64 #80

Closed MrCroxx closed 3 years ago

MrCroxx commented 3 years ago

Previously we use [u8] to define version. It's hard to make sure everything is right (see raft-engine#79).

For now RaftEngine is not GA, it's better change the version format to u64.

IMPORTANT: Make sure that there MUST BE NO user using RaftEngine.

This patch is not compatibility with the previous log file foramt!!!

Signed-off-by: MrCroxx mrcroxx@outlook.com

MrCroxx commented 3 years ago

Right now the version number is nothing more than another magic number. To improve, you can implement a open_file_with_version(version) -> Result<LogFd> and do the version check inside.

@tabokie I think parsing version when opening files is not appropriate. Now when RaftEngine switch to a new log file, it also use fn open_active_file but there's no data yet. IMO, version parsing should be done when actually reading a file.

tabokie commented 3 years ago

@MrCroxx I think during LogFd initialization the version should be checked and the meta info should be saved (e.g. encryption type, compression type, ..), because subsequent foreground requests directly use them without checking. In fact the LogFd should be treated as BlockReader in RocksDB, rather than a fd handle. And I don't think open_active_file is a blocker, you can simply rename it to create_file.