diff --git a/cmapi/cmapi_server/controllers/endpoints.py b/cmapi/cmapi_server/controllers/endpoints.py index 047e452d3..349b160af 100644 --- a/cmapi/cmapi_server/controllers/endpoints.py +++ b/cmapi/cmapi_server/controllers/endpoints.py @@ -913,16 +913,20 @@ class ClusterController: config = request_body.get('config', DEFAULT_MCS_CONF_PATH) in_transaction = request_body.get('in_transaction', False) read_only = request_body.get('read_only', False) + add_other_nodes_dbroots = request_body.get('add_other_nodes_dbroots', True) if node is None: raise_422_error(module_logger, func_name, 'missing node argument') + if not read_only and ('add_other_nodes_dbroots' in request_body): + raise_422_error(module_logger, func_name, 'add_other_nodes_dbroots is only valid when read_only is true') + try: if not in_transaction: with TransactionManager(extra_nodes=[node]): - response = ClusterHandler.add_node(node, config, read_only) + response = ClusterHandler.add_node(node, config, read_only, add_other_nodes_dbroots) else: - response = ClusterHandler.add_node(node, config, read_only) + response = ClusterHandler.add_node(node, config, read_only, add_other_nodes_dbroots) except CMAPIBasicError as err: raise_422_error(module_logger, func_name, err.message) diff --git a/cmapi/cmapi_server/handlers/cluster.py b/cmapi/cmapi_server/handlers/cluster.py index 0dea27c1a..9fbaaa3a5 100644 --- a/cmapi/cmapi_server/handlers/cluster.py +++ b/cmapi/cmapi_server/handlers/cluster.py @@ -50,7 +50,7 @@ def toggle_cluster_state( broadcast_new_config(config, distribute_secrets=True) -class ClusterHandler(): +class ClusterHandler: """Class for handling MCS Cluster operations.""" @staticmethod @@ -143,6 +143,7 @@ class ClusterHandler(): def add_node( node: str, config: str = DEFAULT_MCS_CONF_PATH, read_only: bool = False, + add_other_nodes_dbroots: bool = True, ) -> dict: """Method to add node to MCS CLuster. @@ -151,8 +152,10 @@ class ClusterHandler(): :param config: columnstore xml config file path, defaults to DEFAULT_MCS_CONF_PATH :type config: str, optional - :param read_only: add node in read-only mode, defaults to False + :param read_only: add node in read-only mode (not starting write engine), defaults to False :type read_only: bool, optional + :param add_other_nodes_dbroots: for read-only nodes, whether to add dbroots of other nodes + :type add_other_nodes_dbroots: bool, optional :raises CMAPIBasicError: on exception while starting transaction :raises CMAPIBasicError: if transaction start isn't successful :raises CMAPIBasicError: on exception while adding node @@ -170,11 +173,15 @@ class ClusterHandler(): response = {'timestamp': str(datetime.now())} + if not read_only and add_other_nodes_dbroots is not True: + raise CMAPIBasicError('add_other_nodes_dbroots is only valid when read_only is true') + try: add_node( node, input_config_filename=config, output_config_filename=config, read_only=read_only, + add_other_nodes_dbroots=add_other_nodes_dbroots, ) if not get_dbroots(node, config): if not read_only: # Read-only nodes don't own dbroots diff --git a/cmapi/cmapi_server/node_manipulation.py b/cmapi/cmapi_server/node_manipulation.py index 2fbc5f106..75c6db9df 100644 --- a/cmapi/cmapi_server/node_manipulation.py +++ b/cmapi/cmapi_server/node_manipulation.py @@ -64,6 +64,7 @@ def add_node( output_config_filename: Optional[str] = None, use_rebalance_dbroots: bool = True, read_only: bool = False, + add_other_nodes_dbroots: bool = True, ): """Add node to a cluster. @@ -102,7 +103,7 @@ def add_node( _add_WES(c_root, pm_num, node) else: logging.info('Node is read-only, skipping WES addition.') - _add_read_only_node(c_root, node) + _add_read_only_node(c_root, node, add_other_nodes_dbroots) _add_DBRM_Worker(c_root, node) _add_Module_entries(c_root, node) @@ -113,7 +114,8 @@ def add_node( _rebalance_dbroots(c_root) _move_primary_node(c_root) - update_dbroots_of_readonly_nodes(c_root) + if read_only and add_other_nodes_dbroots: + update_dbroots_of_readonly_nodes(c_root) except Exception: logging.error( 'Caught exception while adding node, config file is unchanged', @@ -1020,8 +1022,8 @@ def _add_WES(root, pm_num, node): etree.SubElement(wes_node, "Port").text = "8630" -def _add_read_only_node(root: etree.Element, node: str) -> None: - '''Add node name to ReadOnlyNodes if it is not already there''' +def _add_read_only_node(root: etree.Element, node: str, add_other_nodes_dbroots: bool = True) -> None: + '''Add node name to ReadOnlyNodes if it is not already there, with attribute''' read_only_nodes = root.find('./ReadOnlyNodes') if read_only_nodes is None: read_only_nodes = etree.SubElement(root, 'ReadOnlyNodes') @@ -1031,9 +1033,11 @@ def _add_read_only_node(root: etree.Element, node: str) -> None: logging.warning( f"_add_read_only_node(): node {node} already exists in ReadOnlyNodes" ) + n.set("add_other_nodes_dbroots", str(add_other_nodes_dbroots).lower()) return - - etree.SubElement(read_only_nodes, "Node").text = node + node_elem = etree.SubElement(read_only_nodes, "Node") + node_elem.text = node + node_elem.set("add_other_nodes_dbroots", str(add_other_nodes_dbroots).lower()) def _add_DBRM_Worker(root, node): @@ -1202,17 +1206,28 @@ def get_pm_module_num_to_addr_map(root: etree.Element) -> dict[int, str]: def update_dbroots_of_readonly_nodes(root: etree.Element) -> None: """Read-only nodes do not have their own dbroots, but they must have all the dbroots of the other nodes So this function sets list of dbroots of each read-only node to the list of all the dbroots in the cluster + + There is a special sub-mode of read-only nodes: coordinator nodes, they don't add dbroots of other nodes. + They are identified by the add_other_nodes_dbroots="false" attribute. """ - nc = NodeConfig() + pm_num_to_addr = get_pm_module_num_to_addr_map(root) - for ro_node in nc.get_read_only_nodes(root): + read_only_nodes = root.find('./ReadOnlyNodes') + if read_only_nodes is None: + return + for n in read_only_nodes.findall("./Node"): + add_dbroots = n.get("add_other_nodes_dbroots", "true").lower() == "true" + if not add_dbroots: + logging.info(f"Not adding dbroots of other nodes to {n.text} because it is a coordinator node") + continue + + ro_node = n.text # Get PM num by IP address this_ip_pm_num = None for pm_num, pm_addr in pm_num_to_addr.items(): if pm_addr == ro_node: this_ip_pm_num = pm_num break - if this_ip_pm_num is not None: # Add dbroots of other nodes to this read-only node add_dbroots_of_other_nodes(root, this_ip_pm_num)