Feature/revised/riak cli handoff status 1239 #661
Conversation
| populate_inbound_types(Inbound, Outbound) -> | ||
| [case find_matching_outbound(Transfer, Outbound) of | ||
| [] -> Transfer; | ||
| [Match | _] -> |
There was a problem hiding this comment.
Throwing away extra list entries seems potentially suspect to me. If find_matching_outbound should only ever return one item, then we should only match on one item. If not, can we safely ignore extra entries? I'm wondering whether we might potentially end up showing the wrong inbound transfer type here. What if there are multiple outbound transfers that have the same target_node/target_partition/mod, but different types? (Not sure what mod means though, so maybe that rules out differing types?)
There was a problem hiding this comment.
I think I did that because I had once seen a literal duplicate in active transfers somewhere, but perhaps I was hallucinating, as I wasn't crazy about it either. I'll try to remove it and do some testing to see what's up, but you really should never have a duplicate active transfer from what I can tell.
JeetKunDoug
force-pushed
the
feature/revised/riak-cli-handoff-status-1239
branch
4 times, most recently
from
750c111 to
b0c12ae
Compare
| handoff_details([], [{all, _Value}]) -> | ||
| build_handoff_details(all); | ||
| handoff_details([], _) -> | ||
| [riak_cli_status:alert([riak_cli_status:text("Cannot use both --all and --node flags at the same time.")])]. |
There was a problem hiding this comment.
Question for @andrewjstone should there be a way in the flag specs to say that flags x and y are mutually exclusive? Future enhancement for riak_cli perhaps?
There was a problem hiding this comment.
Yep @mrallen1 , I just opened an issue for this: basho/clique#22
|
Great work @JeetKunDoug Thanks. |
|
👍 |
JeetKunDoug
force-pushed
the
feature/revised/riak-cli-handoff-status-1239
branch
2 times, most recently
from
80fec5c to
9e3ff06
Compare
addition to riak_core_handoff_status (the main module) the following
changes were made to other modules in support of this functionality.
* Refactored riak_core_vnode_manager:handle_call({repair...}) for readability.
* Refactor riak_core_vnode_manager:determine_handoff_target/4 into a series of function headers
* Create riak_core_vnode_manager:all_handoffs
* Some additional fixes for refactoring of handoff_type/ho_type into
single cohesive type from 07e32b0
JeetKunDoug
force-pushed
the
feature/revised/riak-cli-handoff-status-1239
branch
from
9e3ff06 to
b500a0c
Compare
| case transfer_info(Transfer, stats) of | ||
| no_stats -> unknown; | ||
| Stats -> | ||
| {Key, Value} = lists:keyfind(Key, 1, Stats), |
There was a problem hiding this comment.
Will this ever return false?
There was a problem hiding this comment.
If it does, it should blow up - the list of stats is hard-coded in riak_core_handoff_manager:calc_stats and is always the same.
* Refactor parse_ring_into_known and explode_ring_modules into simpler list comprehensions. * Merge riak_cli registrations from handoff_status module to handoff_cli.
|
👍 8fc16f9 |
…tatus-1239 Feature/revised/riak cli handoff status 1239 Reviewed-by: andrewjstone
…tatus-1239 Feature/revised/riak cli handoff status 1239 Reviewed-by: zeeshanlakhani,aberghage
|
@borshop merge |
Implement "riak-admin handoff summary" and "riak-admin handoff details [--node=] [--all]"