Repair updated for edge case created with new directory structure. #78

Merged
merged 4 commits into from May 17, 2013

Projects

None yet

2 participants

@matthewvon matthewvon was assigned May 9, 2013
@engelsanchez engelsanchez and 1 other commented on an outdated diff May 17, 2013
@@ -115,6 +116,27 @@ class Repairer {
env_->UnlockFile(db_lock_);
}
}
+
+ // perform Riak specific scan for overlapping .sst files
+ // within a level
+ if (status.ok())
+ {
+ leveldb::DB * db_ptr;
+ Options options;
+
+ db_ptr=NULL;
+ options=org_options_;
+ options.block_cache=options_.block_cache;
@engelsanchez
engelsanchez May 17, 2013

Is there any danger in reusing this block cache for the level repair operation here? I haven't fully grasped what the code before this may put in it or where the repair may pull data out of it, but I was wondering if it was safer to just use a fresh one here.

@matthewvon
matthewvon May 17, 2013

The block cache is global to the database (vnode). It's pointer has to be passed around different operations simply to give access. That is what you are seeing here. Good question though.

@engelsanchez

After talking to @matthewvon about this we have agreed to avoid block cache reuse here to avoid any potential current or future weirdness if semantics change. In this case, it is only reused within the repair operation. When used from eleveldb, it is not sharing an existing block cache anyway (so not shared with the Riak vnode). So I'm giving this a 👍 💃 conditional on the code changing to create a new block cache (options block_cache set to NULL). It will be taken away if any other naughtiness is introduced, however :)

@engelsanchez engelsanchez was assigned May 17, 2013
matthewvon added some commits May 17, 2013
@matthewvon matthewvon not reusing block_cache during second phase of repair. looks ok today…
…, but there could be edge cases not seen or future code that would break assumptions.
553fc2b
@matthewvon matthewvon Merge branch 'mv-repair-by-level' of github.com:basho/leveldb into mv…
…-repair-by-level
ddc337f
@matthewvon matthewvon merged commit 43ca5ad into master May 17, 2013
@matthewvon matthewvon deleted the mv-repair-by-level branch May 5, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment