Mikhail Efremov created IGNITE-22774:
----------------------------------------
Summary: Remove raftClient map from ItTxTestCluster
Key: IGNITE-22774
URL: https://issues.apache.org/jira/browse/IGNITE-22774
Project: Ignite
Issue Type: Improvement
Reporter: Mikhail Efremov
Assignee: Mikhail Efremov
*Description*
The goal is to remove {{raftClient}} from {{ItTxTestCluster}} due to all
raft-clients are created inside replcas through
{{ReplicaManager#startReplica}}, and all usages of {{raftClient}} are
effectively useless: this map is used in the single common case:
{code:title=|language=java|collapse=false}assertSame(
txTestCluster.raftClients.get(ACC_TABLE_NAME).get(0).clusterService(),
txTestCluster.getLeader(ACC_TABLE_NAME).service()
);
{code}
Effectively, the first statement get raft-client for a table and it's 0th
partition, then extract it's {{ClusterService}}. The second statement inside of
{{#getLeader}} do the similar thing:
{code:title=|language=java|collapse=false}protected Loza getLeader(String
tableName) {
var services = raftClients.get(tableName);
Peer leader = services.get(0).leader();
assertNotNull(leader);
return raftServers.get(leader.consistentId());
}
{code}
Thus, the second statement means that we get a {{ClusterService}} from {{Loza}}
instance that related to leader's raft-client.
But when we put clients inside the map, we put only raft-clients from leaders
of partition's replication group:
{code:title=|language=java|collapse=false}if (startClient) {
RaftGroupService service = RaftGroupServiceImpl
.start(grpId, client, FACTORY, raftConfig, membersConf, true,
executor, commandsMarshaller)
.get(5, TimeUnit.SECONDS);
clients.put(p, service);
} else {
// Create temporary client to find a leader address.
ClusterService tmpSvc = cluster.get(0);
RaftGroupService service = RaftGroupServiceImpl
.start(grpId, tmpSvc, FACTORY, raftConfig, membersConf, true,
executor, commandsMarshaller)
.get(5, TimeUnit.SECONDS);
Peer leader = service.leader();
service.shutdown();
ClusterService leaderSrv = cluster.stream()
.filter(cluster ->
cluster.topologyService().localMember().name().equals(leader.consistentId()))
.findAny()
.orElseThrow();
RaftGroupService leaderClusterSvc = RaftGroupServiceImpl
.start(grpId, leaderSrv, FACTORY, raftConfig, membersConf, true,
executor, commandsMarshaller)
.get(5, TimeUnit.SECONDS);
clients.put(p, leaderClusterSvc);
}
{code}
The first branch relates to tests that requres separated node as transaction
coordinator and legacy approach demanded for separated raft-client. Now it all
works on replication layer and the first branch will be fully removed from
[https://issues.apache.org/jira/browse/IGNITE-22691]. Then, the second branch
create tempoaral raft-client just to find out a leader peer, and then get
leader's {{ClusterService}} and create an instance of raft-client based on
found service. Again, after beforementioned ticket there wouldn't any
raft-clients instantinations, but still the logic after this branch is get a
leader's raft-client and save it to the map.
Then, the first statement get a leader {{ClusterService}} from the map for
given table and 0th partition, and then the second statement get {{Loza}}
instance for leader's node consistent ID and and ask it for a
{{ClusterService}}. The both services are always identical because it's the
same replication layer leader node. Then, the assertion is redundand and all
the code with {{raftClients}} and {{#leader()}} are the same.
The only place that didn't mention, but shouldn't be removed, but rewritten is
{{getLeaderId}} that is required by
{{ItTxDistributedTestThreeNodesThreeReplicas#testPrimaryReplicaDirectUpdateForExplicitTxn}}
test, returns more explict by method's name {{Peer}} type. The test is
internally bloking message handling on RAFT replication layer before entity
will be written in RAFT-log, and then it must be done exactly on RAFT-group
leader. Then, taking into the account {{raftClients}} removal, the method's
body should be:
{code:title=|language=java|collapse=false}int partId = 0;
reutrn replicaManagers.get(extractConsistentId(cluster.get(partId)))
.replica(new TablePartitionId(tables.get(tableName),
partId))
.thenApply(replica -> replica.raftClient().leader());
{code}
The function {{extractConsistentId(cluster.get(partId))}} is equivalent to
{{cluster.get(partId).topologyService().localMember().name()}} and {{tables}}
is a mapping of tables' names to {{TableImpl}} instances that are inserted to
the map in {{ItTxTestCluster#startTable}} right after instantiation.
*Motivation*
The map and related code are redundand, aren't working as intented and should
be removed.
*Definition of done*
# {{raftClients}} map is removed.
# {{getLeader}} and related assertions are removed.
# {{getLeaderId}} method is rewriten as provided above.
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
