tikv / raft-engine

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

Change `Engine` generic type #106

Closed yuqi1129 closed 3 years ago

yuqi1129 commented 3 years ago

In order to complete the issue Update the version of raft-engine as it has out of date,

Raft-engine can't export Engine::open method, see below

TiKV

    pub fn new(config: RaftEngineConfig) -> Result<Self> {
        Ok(RaftLogEngine(
            RawRaftEngine::open(config).map_err(transfer_error)?,
        ))
    }

Raft-engine

pub type Engine<M, FileBuilder = file_builder::DefaultFileBuilder> = engine::Engine<M, FileBuilder>;

impl<M> Engine<M, DefaultFileBuilder, FilePipeLog<DefaultFileBuilder>>
where
    M: MessageExt,
{
    pub fn open(
        cfg: Config,
    ) -> Result<Engine<M, DefaultFileBuilder, FilePipeLog<DefaultFileBuilder>>> {
        Self::open_with_listeners(cfg, vec![])
    }
}

We need to change

pub type Engine<M, FileBuilder = file_builder::DefaultFileBuilder> = engine::Engine<M, FileBuilder>;

to

pub type Engine<M, FileBuilder = file_builder::DefaultFileBuilder, FilePipe = file_pipe_log::FilePipeLog<file_builder::DefaultFileBuilder>> = engine::Engine<M, FileBuilder, FilePipe>;
tabokie commented 3 years ago

Could you elaborate on why this is needed? I have a PR that updates the raft engine, and it doesn't require exposing more generic parameters, (https://github.com/tikv/tikv/pull/10937/files#diff-2257f21328c8b6ff1fe766f2eb70c427fe7d9ac1552105707d8d96a888b5209f)

yuqi1129 commented 3 years ago

@tabokie Sorry, I do not notice this PR, I have create this issue and try to make raft-engine is update-to-date in tikv, However, Currently raft-engine do not expose open method in Engine, So i create this issue. this seems to be redundant