Conversation
There was a problem hiding this comment.
Code Review
This pull request significantly updates the High Availability documentation to reflect the transition to the Apache Ratis/Raft consensus protocol. Key changes include renaming "Replicas" to "Followers," detailing physical replication and snapshot-based resync, and updating configuration parameters such as the new Raft port (2434) and cluster tokens. Review feedback focused on improving the clarity and consistency of the serverList format description across different documentation files to accurately represent optional positional parameters.
| If not specified, the default server name is `ArcadeDB_0`. | ||
| 1. Enable the HA module by setting the configuration `arcadedb.ha.enabled` to `true`. | ||
| 2. Define the servers that are part of the cluster. | ||
| To set up the server list, define the `arcadedb.ha.serverList` setting as a comma-separated list of servers in the following format: `<hostname/ip-address[:raftPort][:httpPort][:priority]>`. |
There was a problem hiding this comment.
The format description for serverList is slightly ambiguous regarding the optionality of positional parameters. Using nested brackets more clearly indicates that to specify a later parameter (like httpPort), the preceding ones (like raftPort) must also be provided. Also, the current notation [:raftPort][:httpPort] might be interpreted as these being independent optional fields, which is not the case for positional parsing.
To set up the server list, define the `arcadedb.ha.serverList` setting as a comma-separated list of servers in the following format: `<hostname/ip-address>[:<raftPort>[:<httpPort>[:<priority>]]]`.
| |`+arcadedb.ha.replicationIncomingPorts+`|TCP/IP port number (range) used for incoming replication connections|2424-2433 | ||
| |`+arcadedb.ha.enabled+`|True if HA is enabled for the current server|false | ||
| |`+arcadedb.ha.clusterName+`|Cluster name.|arcadedb | ||
| |`+arcadedb.ha.serverList+`|List of <hostname/ip-address:raftPort:httpPort[:priority]> separated by comma.|NOT DEFINED |
There was a problem hiding this comment.
The format description here is inconsistent with the one provided earlier in the document (line 13) and lacks backticks for readability. It also incorrectly implies that colons for raftPort and httpPort are mandatory even if the ports are not provided.
|`+arcadedb.ha.serverList+`|List of `<hostname/ip-address>[:<raftPort>[:<httpPort>[:<priority>]]]` separated by comma.|NOT DEFINED
| |`ha.errorRetries`|Number of automatic retries in case of IO errors with a specific server. If replica servers are configured, the operation will be retried a specific amount of times on the next server in the list. 0 (default) is to retry against all the configured servers|Integer|0 | ||
| |`ha.clusterToken`|Shared secret for inter-node command authentication. Must be identical on all nodes.|String|null | ||
| |`ha.raftPort`|Default TCP/IP port for Raft gRPC communication.|Integer|2434 | ||
| |`ha.serverList`|Servers in the cluster as a list of <hostname/ip-address:raftPort:httpPort[:priority]> items separated by comma.|String| |
There was a problem hiding this comment.
The format description for ha.serverList should be consistent with the HA operations guide. The current version implies that raftPort and httpPort are mandatory, which contradicts the default values mentioned elsewhere.
|`ha.serverList`|Servers in the cluster as a list of `<hostname/ip-address>[:<raftPort>[:<httpPort>[:<priority>]]]` items separated by comma.|String|
No description provided.