tikv / raft-engine

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

Optimize the cold startup of RaftEngine when `enable-log-recycle` == true #278

Closed LykxSassinator closed 1 year ago

LykxSassinator commented 2 years ago

Discription

This commit optimize the startup of Engine when enable-log-recycle == true, by preparing a bunch of logs named with special suffix ".fakelog", so-called "Fake logs", to curtail the side effect when there is no enough stale files for log recycling.

Related Issue: close #277

Signed-off-by: Lucasliang nkcs_lykx@hotmail.com

codecov[bot] commented 2 years ago

Codecov Report

Base: 97.74% // Head: 97.73% // Decreases project coverage by -0.01% :warning:

Coverage data is based on head (459c127) compared to base (3353011). Patch coverage: 97.88% of modified lines in pull request are covered.

Additional details and impacted files ```diff @@ Coverage Diff @@ ## master #278 +/- ## ========================================== - Coverage 97.74% 97.73% -0.01% ========================================== Files 30 30 Lines 11287 11411 +124 ========================================== + Hits 11032 11153 +121 - Misses 255 258 +3 ``` | [Impacted Files](https://codecov.io/gh/tikv/raft-engine/pull/278?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=tikv) | Coverage Δ | | |---|---|---| | [src/file\_pipe\_log/mod.rs](https://codecov.io/gh/tikv/raft-engine/pull/278?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=tikv#diff-c3JjL2ZpbGVfcGlwZV9sb2cvbW9kLnJz) | `98.46% <ø> (ø)` | | | [src/pipe\_log.rs](https://codecov.io/gh/tikv/raft-engine/pull/278?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=tikv#diff-c3JjL3BpcGVfbG9nLnJz) | `95.45% <ø> (ø)` | | | [tests/failpoints/mod.rs](https://codecov.io/gh/tikv/raft-engine/pull/278?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=tikv#diff-dGVzdHMvZmFpbHBvaW50cy9tb2QucnM=) | `100.00% <ø> (ø)` | | | [tests/failpoints/util.rs](https://codecov.io/gh/tikv/raft-engine/pull/278?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=tikv#diff-dGVzdHMvZmFpbHBvaW50cy91dGlsLnJz) | `98.86% <ø> (-0.09%)` | :arrow_down: | | [src/file\_pipe\_log/format.rs](https://codecov.io/gh/tikv/raft-engine/pull/278?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=tikv#diff-c3JjL2ZpbGVfcGlwZV9sb2cvZm9ybWF0LnJz) | `99.10% <94.44%> (-0.41%)` | :arrow_down: | | [src/file\_pipe\_log/pipe\_builder.rs](https://codecov.io/gh/tikv/raft-engine/pull/278?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=tikv#diff-c3JjL2ZpbGVfcGlwZV9sb2cvcGlwZV9idWlsZGVyLnJz) | `95.49% <96.12%> (-0.12%)` | :arrow_down: | | [src/file\_pipe\_log/pipe.rs](https://codecov.io/gh/tikv/raft-engine/pull/278?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=tikv#diff-c3JjL2ZpbGVfcGlwZV9sb2cvcGlwZS5ycw==) | `98.48% <96.81%> (-1.01%)` | :arrow_down: | | [src/config.rs](https://codecov.io/gh/tikv/raft-engine/pull/278?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=tikv#diff-c3JjL2NvbmZpZy5ycw==) | `97.57% <100.00%> (+0.24%)` | :arrow_up: | | [src/engine.rs](https://codecov.io/gh/tikv/raft-engine/pull/278?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=tikv#diff-c3JjL2VuZ2luZS5ycw==) | `98.29% <100.00%> (+0.09%)` | :arrow_up: | | [src/env/default.rs](https://codecov.io/gh/tikv/raft-engine/pull/278?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=tikv#diff-c3JjL2Vudi9kZWZhdWx0LnJz) | `93.40% <100.00%> (+0.58%)` | :arrow_up: | | ... and [9 more](https://codecov.io/gh/tikv/raft-engine/pull/278?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=tikv) | | Help us with your feedback. Take ten seconds to tell us [how you rate us](https://about.codecov.io/nps?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=tikv). Have a feature suggestion? [Share it here.](https://app.codecov.io/gh/feedback/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=tikv)

:umbrella: View full report at Codecov.
:loudspeaker: Do you have feedback about the report comment? Let us know in this issue.

LykxSassinator commented 2 years ago

@tabokie PTAL, thx

LykxSassinator commented 2 years ago

@tabokie PTAL, thx

LykxSassinator commented 1 year ago

@tabokie PTAL, thx.

LykxSassinator commented 1 year ago

To improve, I think you can just keep thing even simpler, make FileCollection as simple as possible (as simple as a Vec):

Thx for your advices. I've also take this design in previous draft, but I dropped it finally. For the reason why I didn't adopt the structure as u said, just like:

struct FileCollection {
    stale_files: FileList<F>,
    active_files: FileList<F>,
    ...
}

is that: In previous work, SinglePipe is the operator and manager of FileCollection. IMO, the Stale files and Active files, represent by StaleFileCollection and ActiveFileCollection respectively, are the minimal operating unit of FileList. So, each one of them should be designed as an individual object because the related operations has been encapsulated in the API of SinglePipe. So, from my views, SinglePipe is just the subject with same trait as the new FileCollection u mentioned above. For example, SinglePipe.get_fd(...) just called the ActiveFileCollection.get_fd(...) to return the expected fd to callers.

And the confusing parts, such as prepare_dummy_logs_for_recycle, will be polished.

tabokie commented 1 year ago

@LykxSassinator It doesn't contradict with that I said.

Then all the complicated logic can be put in-place, or wrapped into a bigger struct:

The code snippet I wrote refers to the way to "wrapped into a bigger struct". It can be done as "put in-place" as well.

LykxSassinator commented 1 year ago

@LykxSassinator It doesn't contradict with that I said.

Then all the complicated logic can be put in-place, or wrapped into a bigger struct:

The code snippet I wrote refers to the way to "wrapped into a bigger struct". It can be done as "put in-place" as well.

Yes, i got what u meaned. I will polish the current codes.

LykxSassinator commented 1 year ago

ping @tabokie PTAL, thx

LykxSassinator commented 1 year ago

Ping @tabokie

LykxSassinator commented 1 year ago

Ping @tabokie

LykxSassinator commented 1 year ago

Ping @tabokie ,thx

LykxSassinator commented 1 year ago

Wait for final review after #285 is merged.

LykxSassinator commented 1 year ago

Wait for final review after #285 is merged.

Ignore, related updates of the dependency on rust version have been updated in this pr.

LykxSassinator commented 1 year ago

@tabokie ping.

LykxSassinator commented 1 year ago

ping @tabokie PTAL, thx.

LykxSassinator commented 1 year ago

I took the liberty of cleaning up the abstraction myself, it grows to be, somewhat superfluous IMO.

And by the way, #278 (comment) is not addressed. When you use &mut self, there's no need to lock again, because mutable reference guarantees exclusive access.

Thx for your suggestions. I'll take some time to comprehend the newly introduced refactoring works.

LykxSassinator commented 1 year ago

ping @tabokie PTAL, thx.

LykxSassinator commented 1 year ago

ping @tabokie, thx.