Details
-
Bug
-
Resolution: Fixed
-
Major
-
5.1.0, 5.5.0
-
Untriaged
-
Unknown
Description
From experimentation I noticed that it was possible to change the index path of a running node (which already has indexes).
A trivial reproduction case is to do the following:
- Create a single-node cluster with query+index enabled, do not alter the paths.
- Create a few indexes
- Create the directory '/opt/couchbase/var/lib/couchbase/index' and give ownership to couchbase:couchbase.
- Run the following couchbase-cli command to change the index path:
/opt/couchbase/bin/couchbase-cli node-init -c localhost:8091 -u Administrator -p password --node-init-index-path="/opt/couchbase/var/lib/couchbase/index"
- The indexer will then restart (due to ns_server closing its goports as part of the restart) and pickup the newly specified path and all previous indexes will be 'lost'.
While this is not a very common scenario, there are definitely situations where this could happen, for example misunderstanding the node initialization API/sending the wrong hostname or IP/bad automation.
Reviewing the code (vulcan, although pre-vulcan is mostly the same) it's because we are only checking if the database path has changed:
{ok, DefaultDbPath} = ns_storage_conf:this_node_dbdir(), |
DbPathChanged = DbPath =/= DefaultDbPath, |
Provisioned = ns_config_auth:is_system_provisioned(), |
NodeStatus = ns_cluster_membership:get_cluster_membership(node()), |
|
case DbPathChanged and Provisioned and (NodeStatus =/= inactiveAdded) of |
true ->
|
%% MB-7344: we had 1.8.1 instructions allowing that. And |
%% 2.0 works very differently making that original |
%% instructions lose data. Thus we decided it's much safer |
%% to un-support this path. |
Msg = "Changing data of nodes that are part of provisioned " |
"cluster is not supported", |
erlang:throw({error, [Msg]}); |
_ ->
|
ok
|
end, |
Perhaps I'm looking at the situation naively, but why don't we just prevent path changes if the node is provisioned and not inactiveAdded? I can't think of a legitimate scenario where you would want to change the index or CBAS path of an active node.
If there's a reason that the API is designed this way then I'm happy to accept that, I just can't personally see what benefits it's adding compared to the potential disastrous consequences.