Fix reip loading of riak_core #977
Conversation
| case application:load(riak_core) of | ||
| %% a process, such as cuttlefish, may have already loaded riak_core | ||
| {error,{already_loaded,riak_core}} -> ok; | ||
| ok -> ok |
There was a problem hiding this comment.
As I understand it, we can safely ignore {error, {already_loaded, riak_core}}. However, the application:load/1 function specifies {error, Reason }. How should {error, _} be handled?
There was a problem hiding this comment.
I'm going to allow any other error to just break and bubble to the surface. Adding the {error, _} case gives us two options: set ok and continue, which would just push the problem to line 225. Or, throw a different error, which is immediately caught by this function anyway.
There was a problem hiding this comment.
@coderoshi seems reasonable to me. +1
|
Wasn't there a decision to remove reip for 2.0? "cluster replace" does the same thing, but correctly. |
|
We decided not to - the case it doesn't handle is a restore from backup On Tue, Jun 10, 2014 at 10:18 PM, Sean Cribbs notifications@github.com
Jon Meredith |
|
@coderoshi When testing I noticed errors weren't bubbling up to the console. With reip being run when the node is down, the lager:error call in this catch won't be written: https://github.com/basho/riak_kv/blob/bug/er/reip-fix/src/riak_kv_console.erl#L236. Can this be changed to a single io:format, outputting the exception and reason, as part of this PR? |
|
Agreed @jcapricebasho. Also, rebased |
|
Just for record, this is reproducible in at least 2.0.0beta6, which is where I found the issue. It can be reproduced by building a devrel, stopping a node, and attempting to run a |
|
Code looks good, and test passed on my laptop. +1. |
|
@jburwell want to +1 for bors? |
|
+1 574f42a Sorry for the n00b mistake. |
Fix reip loading of riak_core Reviewed-by: jburwell
|
@borshop merge |
Fixes basho/riak#554, reip throwing an a badmatch when attempting to load riak_core, which was already loaded by cuttlefish.