sanpwc commented on code in PR #1655:
URL: https://github.com/apache/ignite-3/pull/1655#discussion_r1105562451
##########
modules/table/src/main/java/org/apache/ignite/internal/table/distributed/TableManager.java:
##########
@@ -1863,23 +1863,36 @@ public void onUpdate(WatchEvent evt) {
if (zoneId == tableZoneId) {
TableConfiguration tableCfg =
tables.get(tableView.name());
+ byte[] assignmentsBytes =
((ExtendedTableConfiguration) tableCfg).assignments().value();
+
+ List<Set<Assignment>> tableAssignments =
ByteUtils.fromBytes(assignmentsBytes);
+
for (int part = 0; part < tableView.partitions();
part++) {
UUID tableId = ((ExtendedTableConfiguration)
tableCfg).id().value();
TablePartitionId replicaGrpId = new
TablePartitionId(tableId, part);
- int partId = part;
+ int replicas = tableView.replicas();
- updatePendingAssignmentsKeys(
- tableView.name(), replicaGrpId,
dataNodes, tableView.replicas(),
-
evt.entryEvent().newEntry().revision(), metaStorageMgr, part
- ).exceptionally(e -> {
- LOG.error(
- "Exception on updating assignments
for [table={}, partition={}]", e, tableView.name(), partId
- );
+ Set<Assignment> partAssignments =
AffinityUtils.calculateAssignmentForPartition(dataNodes, part, replicas);
Review Comment:
So, we will recalculate partAssignments twice, here and inside
updatePendingAssignmentsKeys. Is it possible to have one calculation only?
##########
modules/table/src/main/java/org/apache/ignite/internal/table/distributed/TableManager.java:
##########
@@ -1863,23 +1863,36 @@ public void onUpdate(WatchEvent evt) {
if (zoneId == tableZoneId) {
TableConfiguration tableCfg =
tables.get(tableView.name());
+ byte[] assignmentsBytes =
((ExtendedTableConfiguration) tableCfg).assignments().value();
+
+ List<Set<Assignment>> tableAssignments =
ByteUtils.fromBytes(assignmentsBytes);
+
for (int part = 0; part < tableView.partitions();
part++) {
UUID tableId = ((ExtendedTableConfiguration)
tableCfg).id().value();
TablePartitionId replicaGrpId = new
TablePartitionId(tableId, part);
- int partId = part;
+ int replicas = tableView.replicas();
- updatePendingAssignmentsKeys(
- tableView.name(), replicaGrpId,
dataNodes, tableView.replicas(),
-
evt.entryEvent().newEntry().revision(), metaStorageMgr, part
- ).exceptionally(e -> {
- LOG.error(
- "Exception on updating assignments
for [table={}, partition={}]", e, tableView.name(), partId
- );
+ Set<Assignment> partAssignments =
AffinityUtils.calculateAssignmentForPartition(dataNodes, part, replicas);
Review Comment:
Why we check this only for assignments recalculation triggered by dataNodes
change? Is it possible to face same issue in case of other
updatePendingAssignmentsKeys triggers?
##########
modules/table/src/main/java/org/apache/ignite/internal/table/distributed/TableManager.java:
##########
@@ -1863,23 +1863,36 @@ public void onUpdate(WatchEvent evt) {
if (zoneId == tableZoneId) {
TableConfiguration tableCfg =
tables.get(tableView.name());
+ byte[] assignmentsBytes =
((ExtendedTableConfiguration) tableCfg).assignments().value();
+
+ List<Set<Assignment>> tableAssignments =
ByteUtils.fromBytes(assignmentsBytes);
+
for (int part = 0; part < tableView.partitions();
part++) {
UUID tableId = ((ExtendedTableConfiguration)
tableCfg).id().value();
TablePartitionId replicaGrpId = new
TablePartitionId(tableId, part);
- int partId = part;
+ int replicas = tableView.replicas();
- updatePendingAssignmentsKeys(
- tableView.name(), replicaGrpId,
dataNodes, tableView.replicas(),
-
evt.entryEvent().newEntry().revision(), metaStorageMgr, part
- ).exceptionally(e -> {
- LOG.error(
- "Exception on updating assignments
for [table={}, partition={}]", e, tableView.name(), partId
- );
+ Set<Assignment> partAssignments =
AffinityUtils.calculateAssignmentForPartition(dataNodes, part, replicas);
+
+ // TODO IGNITE-18624 This check will not be
needed when dataNodes from distribution zone will be used
+ // instead of BaselineManager.nodes.
+ if
(!partAssignments.equals(tableAssignments.get(part))) {
+ int partId = part;
- return null;
- });
+ updatePendingAssignmentsKeys(
Review Comment:
What if we already have pending rebalance and should add calculated ones as
planned ones? I mean following:
current A,B,C
pending A,B
new one that we've calculated, again A,B,C
despite the fact that current == new one we shouldn't skip it.
Please pay attention, that we have the logic similar to the one we want
inside updatePendingAssignmentsKeys that checks assignments.stable and new
assignments equality
`&& partition.assignments.stable != calcPartAssighments()`
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
