MCOL-5861 cmapi read only nodes #3432

mariadb-AlexanderPresniakov · 2025-03-12T13:23:14Z

https://jira.mariadb.org/browse/MCOL-5861
Read-only nodes don't start WriteEngineServer and do not own dbroots. But as read-only nodes must have RO access to all the data in the cluster, all dbroots in the cluster are assigned all the dbroots.
Read-only nodes are written into ReadOnlyNodes part of the config.
On every change that could add or remove dbroots update_dbroots_of_readonly_nodes is called to assign all existing dbroots to every read-only node.

MCSProgs was added to avoid possible typos in service names (because there is no single naming convention between services and it is very easy to make an error)
Also added NodeConfig.modify_config context manager that reads config from specified path, provides its root and on (only successful exit) writes config to the output path

mariadb-AlanMologorsky · 2025-03-29T02:22:14Z

Name branch as a feature/MCOL-.....
And the same about unified commits pls.

Short description is better than just JIRA link. Even summary from commit\s message.

cmapi/cmapi_server/failover_agent.py

cmapi/cmapi_server/handlers/cluster.py

cmapi/cmapi_server/helpers.py

cmapi/cmapi_server/node_manipulation.py

cmapi/mcs_node_control/models/node_config.py

mariadb-AlanMologorsky · 2025-03-29T04:50:53Z

And one thing about NEW readonly node naming - Seems should be discussed together with @drrtuy or @mariadb-LeonidFedorov or both.
Because comparing to any "readonly" mention in previous code this naming could be confusing.

* feat(cmapi): add read_only param for API add node endpoint * style(cmapi): fixes for string length and quotes

mariadb-AlanMologorsky · 2025-04-30T16:20:08Z

cmapi/cmapi_server/logging_management.py

@@ -104,7 +104,8 @@ def enable_console_logging(logger: logging.Logger) -> None:
 def config_cmapi_server_logging():
    # add custom level TRACE only for develop purposes
    # could be activated using API endpoints or cli tool without relaunching
-    add_logging_level('TRACE', 5)
+    if not hasattr(logging, 'TRACE'):
+        add_logging_level('TRACE', 5)


JFYI I think to remove or rewrite this dev function.

mariadb-AlanMologorsky · 2025-04-30T16:23:43Z

cmapi/cmapi_server/managers/process.py

@@ -89,7 +98,8 @@ def _detect_processes(cls) -> None:
        if cls.mcs_progs:
            logging.warning('Mcs ProcessHandler already detected processes.')

-        for prog_name, prog_info in ALL_MCS_PROGS.items():
+        for prog, prog_info in ALL_MCS_PROGS.items():
+            prog_name = prog.value


JFYI It's what I mentioned before here about naming and access names. prog.value == prog_name. It's somehow confusing.

mariadb-AlanMologorsky · 2025-04-30T16:27:59Z

cmapi/cmapi_server/node_manipulation.py

+    CMAPI_CONF_PATH,
+    CMAPI_SINGLE_NODE_XML,
+    DEFAULT_MCS_CONF_PATH,
+    LOCALHOSTS,


Why do we need it on a separate lines?

mariadb-AlanMologorsky · 2025-04-30T16:41:54Z

cmapi/cmapi_server/node_manipulation.py

+
+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


Seems to be vice versa.
sets list of all the dbroots in the cluster to the list of dbroots of each read-only node

mariadb-AlanMologorsky · 2025-04-30T16:46:17Z

cmapi/cmapi_server/node_manipulation.py

+        )
+        dbroot_id_node.text = str(dbroot_id)
+
+    logging.info("Added %d dbroots to read-only node %d: %s", len(existing_dbroots), module_num, sorted(existing_dbroots))


Pls f-string notation.

mariadb-AlanMologorsky · 2025-04-30T18:16:23Z

cmapi/cmapi_server/test/test_node_manip.py

 logging.basicConfig(level='DEBUG')

+SINGLE_NODE_XML = "./cmapi_server/SingleNode.xml"


JFYI I'm not sure that default Columnstore.xml is fully equal to SingleNode.xml

mariadb-AlanMologorsky · 2025-04-30T18:19:04Z

cmapi/cmapi_server/test/test_node_manip.py

-        )
+
+        with patch('cmapi_server.node_manipulation.update_dbroots_of_readonly_nodes') as mock_update_dbroots_of_readonly_nodes:
+            node_manipulation.add_node(


With this mock test name is not fully valid. Here we test add\remove node and mocked function call once.
May be better to move it in separate test?

mariadb-AlanMologorsky · 2025-04-30T18:20:34Z

cmapi/cmapi_server/test/test_node_manip.py

+                self.NEW_NODE_NAME, self.tmp_files[1], self.tmp_files[2],
+                test_mode=True
+            )
+            mock_update_dbroots_of_readonly_nodes.assert_called_once()


It's not clear to me what we trying to test here several times. Several times test that we call the mocked function once?

mariadb-AlanMologorsky · 2025-04-30T18:35:49Z

cmapi/mcs_node_control/models/node_config.py

@@ -137,6 +138,26 @@ def write_config(self, tree, filename=DEFAULT_MCS_CONF_PATH):
            f.write(self.to_string(tree))
        replace(tmp_filename, filename)    # atomic replacement

+    @contextmanager


I'd like to move it in separate PR and integrate in other places tho.

mariadb-AlanMologorsky · 2025-04-30T18:44:20Z

cmapi/pyproject.toml

Why do we need this file?

mariadb-AlexanderPresniakov marked this pull request as draft March 12, 2025 13:23

mariadb-AlexanderPresniakov force-pushed the MCOL-5806-cmapi-read-only-nodes branch 8 times, most recently from bbd12b4 to 728d94d Compare March 18, 2025 12:54

mariadb-AlexanderPresniakov force-pushed the MCOL-5806-cmapi-read-only-nodes branch 3 times, most recently from e698ef4 to a74b938 Compare March 28, 2025 03:53

mariadb-AlexanderPresniakov requested a review from mariadb-AlanMologorsky March 28, 2025 03:53

mariadb-AlexanderPresniakov marked this pull request as ready for review March 28, 2025 03:55

mariadb-AlexanderPresniakov changed the title ~~MCOL-5806 cmapi read only nodes~~ MCOL-5861 cmapi read only nodes Mar 28, 2025

mariadb-AlanMologorsky requested changes Mar 29, 2025

View reviewed changes

mariadb-AlexanderPresniakov force-pushed the MCOL-5806-cmapi-read-only-nodes branch from a74b938 to 220f03f Compare March 31, 2025 19:39

mariadb-AlanMologorsky force-pushed the MCOL-5806-cmapi-read-only-nodes branch from 3ca8a6f to 2bd07e4 Compare April 2, 2025 18:53

mariadb-AlexanderPresniakov force-pushed the MCOL-5806-cmapi-read-only-nodes branch 3 times, most recently from 481d491 to 44ae561 Compare April 14, 2025 15:31

mariadb-AlexanderPresniakov force-pushed the MCOL-5806-cmapi-read-only-nodes branch 2 times, most recently from 4ce4e78 to 1833abc Compare April 23, 2025 19:32

mariadb-AlexanderPresniakov and others added 5 commits April 28, 2025 15:20

MCOL-5806: added ability to start node in read-only mode

37453ad

Part of review changes to retrigger package build

9a1ac0e

feat(cmapi): MCOL-5806 Review and rebase interface fixes.

93cd935

* feat(cmapi): add read_only param for API add node endpoint * style(cmapi): fixes for string length and quotes

Zero-diff to trigger package rebuild

fbf4844

Remove WES stop exclusion logic

3671d49

mariadb-AlexanderPresniakov added 7 commits April 28, 2025 15:20

Zero-diff to retrigger build

1714bef

Add dbroots of other nodes to the read-only node

7255340

Review fixes and ruff settings

04015ae

More review fixes

245cceb

Zero-diff to trigger build

7199c97

Restore edits after cherry-pick

ff25d4e

On every node change adjust dbroots in the read-only nodes

5359c98

mariadb-AlexanderPresniakov force-pushed the MCOL-5806-cmapi-read-only-nodes branch from db91612 to 5359c98 Compare April 28, 2025 15:20

Fix logging (trace level) in tests

e7fd6fc

mariadb-AlexanderPresniakov force-pushed the MCOL-5806-cmapi-read-only-nodes branch from a73579a to e7fd6fc Compare April 28, 2025 19:21

mariadb-AlanMologorsky requested changes Apr 30, 2025

View reviewed changes

		logging.basicConfig(level='DEBUG')

		SINGLE_NODE_XML = "./cmapi_server/SingleNode.xml"

MCOL-5861 cmapi read only nodes #3432

Are you sure you want to change the base?

MCOL-5861 cmapi read only nodes #3432

Uh oh!

Conversation

mariadb-AlexanderPresniakov commented Mar 12, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

mariadb-AlanMologorsky commented Mar 29, 2025

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

mariadb-AlanMologorsky commented Mar 29, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

mariadb-AlexanderPresniakov commented Mar 12, 2025 •

edited

Loading