You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@ignite.apache.org by GitBox <gi...@apache.org> on 2020/06/29 14:15:21 UTC
[GitHub] [ignite] ascherbakoff commented on a change in pull request #7722: IGNITE-12935 Disadvantages in log of historical rebalance
ascherbakoff commented on a change in pull request #7722:
URL: https://github.com/apache/ignite/pull/7722#discussion_r446989868
##########
File path: modules/core/src/main/java/org/apache/ignite/internal/processors/cache/distributed/dht/preloader/GridDhtPartitionsExchangeFuture.java
##########
@@ -4076,9 +4101,60 @@ else if (resetOwners)
throw new IgniteException("Failed to assign partition states", e);
}
+ if (log.isInfoEnabled() && !F.isEmpty(supplyInfoMap))
+ printPartitionRebalancingFully(supplyInfoMap);
+
timeBag.finishGlobalStage("Assign partitions states");
}
+ /**
+ * Prints detail information about partitions which did not have reservation
+ * history enough for historical rebalance.
+ *
+ * @param supplyInfoMap Map contains information about supplying partitions.
+ */
+ private void printPartitionRebalancingFully(Map<String, List<SupplyPartitionInfo>> supplyInfoMap) {
Review comment:
I suggest to wrap printing code to try - catch block to avoid node failure if printing code has errors.
##########
File path: modules/core/src/main/java/org/apache/ignite/internal/processors/cache/distributed/dht/preloader/GridDhtPartitionsExchangeFuture.java
##########
@@ -4076,9 +4101,60 @@ else if (resetOwners)
throw new IgniteException("Failed to assign partition states", e);
}
+ if (log.isInfoEnabled() && !F.isEmpty(supplyInfoMap))
+ printPartitionRebalancingFully(supplyInfoMap);
+
timeBag.finishGlobalStage("Assign partitions states");
}
+ /**
+ * Prints detail information about partitions which did not have reservation
+ * history enough for historical rebalance.
+ *
+ * @param supplyInfoMap Map contains information about supplying partitions.
+ */
+ private void printPartitionRebalancingFully(Map<String, List<SupplyPartitionInfo>> supplyInfoMap) {
+ if (hasPartitonToLog(supplyInfoMap, false)) {
+ log.info("Partitions weren't present in any history reservation: [" +
+ supplyInfoMap.entrySet().stream().map(entry ->
+ "[grp=" + entry.getKey() + " part=[" + S.compact(entry.getValue().stream()
+ .filter(info -> !info.isHistoryReserved())
+ .map(info -> info.part()).collect(Collectors.toSet())) + "]]"
+ ).collect(Collectors.joining(", ")) + ']');
+ }
+
+ if (hasPartitonToLog(supplyInfoMap, true)) {
+ log.info("Partitions were reserved, but maximum available counter is greater than demanded: [" +
+ supplyInfoMap.entrySet().stream().map(entry ->
+ "[grp=" + entry.getKey() + ' ' +
+ entry.getValue().stream().filter(SupplyPartitionInfo::isHistoryReserved).map(info ->
+ "[part=" + info.part() +
+ ", minCntr=" + info.minCntr() +
+ ", maxReserved=" + info.maxReserved() +
+ ", maxReservedNodeId=" + info.maxReservedNodeId() + ']'
+ ).collect(Collectors.joining(", ")) + ']'
+ ).collect(Collectors.joining(", ")) + ']');
+ }
+ }
+
+ /**
+ * Does information contain partitions which will print to log.
+ *
+ * @param supplayInfoMap Map contains information about supplying partitions.
+ * @param reserved Reservation flag.
+ * @return True if map has partitions with same reserved flag, false otherwise.
+ */
+ private boolean hasPartitonToLog(Map<String, List<SupplyPartitionInfo>> supplayInfoMap, boolean reserved) {
Review comment:
typos in hasPartitonToLog, supplayInfoMap.
##########
File path: modules/core/src/main/java/org/apache/ignite/internal/processors/cache/distributed/dht/preloader/GridDhtPartitionsExchangeFuture.java
##########
@@ -4076,9 +4101,60 @@ else if (resetOwners)
throw new IgniteException("Failed to assign partition states", e);
}
+ if (log.isInfoEnabled() && !F.isEmpty(supplyInfoMap))
Review comment:
Enough to check supplyInfoMap != null
##########
File path: modules/core/src/main/java/org/apache/ignite/internal/processors/cache/persistence/GridCacheDatabaseSharedManager.java
##########
@@ -1750,10 +1751,18 @@ private boolean safeToUpdatePageMemories() {
Map</*grpId*/Integer, Map</*partId*/Integer, /*updCntr*/Long>> grpPartsWithCnts = new HashMap<>();
- for (Map.Entry<Integer, Map<Integer, CheckpointEntry>> e : earliestValidCheckpoints.entrySet()) {
+ for (Map.Entry<Integer, T2</*reason*/ReservationReason, Map</*partId*/Integer, CheckpointEntry>>> e : earliestValidCheckpoints.entrySet()) {
int grpId = e.getKey();
- for (Map.Entry<Integer, CheckpointEntry> e0 : e.getValue().entrySet()) {
+ if (e.getValue().get2() == null) {
Review comment:
Looks like it's already logged in printReservationToLog, why we need to log it again in debug ?
##########
File path: modules/core/src/main/java/org/apache/ignite/internal/processors/cache/persistence/GridCacheDatabaseSharedManager.java
##########
@@ -1772,9 +1781,60 @@ private boolean safeToUpdatePageMemories() {
}
}
+ if (log.isInfoEnabled() && !F.isEmpty(earliestValidCheckpoints))
+ printReservationToLog(earliestValidCheckpoints);
+
return grpPartsWithCnts;
}
+ /**
+ * Prints detail information about caches which were not reserved
+ * and reservation depth for the caches which have WAL history enough.
+ *
+ * @param earliestValidCheckpoints Map contains information about caches' reservation.
+ */
+ private void printReservationToLog(
+ Map<Integer, T2<ReservationReason, Map<Integer, CheckpointEntry>>> earliestValidCheckpoints) {
+
+ Map<ReservationReason, List<Integer>> notReservedCachesToPrint = new HashMap<>();
+ Map<ReservationReason, List<T2<Integer, CheckpointEntry>>> reservedCachesToPrint = new HashMap<>();
+
+ for (Map.Entry<Integer, T2<ReservationReason, Map<Integer, CheckpointEntry>>> entry : earliestValidCheckpoints.entrySet()) {
+ if (entry.getValue().get2() == null) {
+ notReservedCachesToPrint.computeIfAbsent(entry.getValue().get1(), reason -> new ArrayList<>())
+ .add(entry.getKey());
+ }
+ else {
+ reservedCachesToPrint.computeIfAbsent(entry.getValue().get1(), reason -> new ArrayList<>())
+ .add(new T2(entry.getKey(), entry.getValue().get2().values().stream().min((cp1, cp2) ->
+ Long.compare(cp1.timestamp(), cp2.timestamp())).get()));
+ }
+ }
+
+ if (!F.isEmpty(notReservedCachesToPrint)) {
+ log.info("Following caches were not reserved [" +
+ notReservedCachesToPrint.entrySet().stream()
+ .map(entry -> '[' +
+ entry.getValue().stream().map(grpId -> "[grpId=" + grpId +
+ ", grpName=" + cctx.cache().cacheGroup(grpId).cacheOrGroupName() + ']')
+ .collect(Collectors.joining(", ")) +
+ ", reason=" + entry.getKey() + ']')
+ .collect(Collectors.joining(", ")) + ']');
+ }
+
+ if (!F.isEmpty(reservedCachesToPrint)) {
+ log.info("Reserved cache groups with first reserved checkpoint IDs and reasons why previous checkpoint was inapplicable: [" +
Review comment:
Reserved cache groups with first reserved checkpoint IDs and reasons why previous checkpoint was inapplicable
->
Cache groups with earliest reserved checkpoint and a reason why a previous checkpoint was inapplicable
##########
File path: modules/core/src/main/java/org/apache/ignite/internal/processors/cache/distributed/dht/preloader/GridDhtPartitionsExchangeFuture.java
##########
@@ -3457,7 +3463,19 @@ private void assignHistoricalSuppliers(
deepestReserved.set(e0.getKey(), histCntr);
}
}
+
+ //No one reservation matched for this partition.
Review comment:
Missing space
##########
File path: modules/core/src/main/java/org/apache/ignite/internal/processors/cache/persistence/GridCacheDatabaseSharedManager.java
##########
@@ -1772,9 +1781,60 @@ private boolean safeToUpdatePageMemories() {
}
}
+ if (log.isInfoEnabled() && !F.isEmpty(earliestValidCheckpoints))
+ printReservationToLog(earliestValidCheckpoints);
+
return grpPartsWithCnts;
}
+ /**
+ * Prints detail information about caches which were not reserved
+ * and reservation depth for the caches which have WAL history enough.
+ *
+ * @param earliestValidCheckpoints Map contains information about caches' reservation.
+ */
+ private void printReservationToLog(
Review comment:
Wrap printing code to try - catch block to avoid node failures.
##########
File path: modules/core/src/main/java/org/apache/ignite/internal/processors/cache/persistence/GridCacheDatabaseSharedManager.java
##########
@@ -1772,9 +1781,60 @@ private boolean safeToUpdatePageMemories() {
}
}
+ if (log.isInfoEnabled() && !F.isEmpty(earliestValidCheckpoints))
+ printReservationToLog(earliestValidCheckpoints);
+
return grpPartsWithCnts;
}
+ /**
+ * Prints detail information about caches which were not reserved
+ * and reservation depth for the caches which have WAL history enough.
+ *
+ * @param earliestValidCheckpoints Map contains information about caches' reservation.
+ */
+ private void printReservationToLog(
+ Map<Integer, T2<ReservationReason, Map<Integer, CheckpointEntry>>> earliestValidCheckpoints) {
+
+ Map<ReservationReason, List<Integer>> notReservedCachesToPrint = new HashMap<>();
+ Map<ReservationReason, List<T2<Integer, CheckpointEntry>>> reservedCachesToPrint = new HashMap<>();
+
+ for (Map.Entry<Integer, T2<ReservationReason, Map<Integer, CheckpointEntry>>> entry : earliestValidCheckpoints.entrySet()) {
+ if (entry.getValue().get2() == null) {
+ notReservedCachesToPrint.computeIfAbsent(entry.getValue().get1(), reason -> new ArrayList<>())
+ .add(entry.getKey());
+ }
+ else {
+ reservedCachesToPrint.computeIfAbsent(entry.getValue().get1(), reason -> new ArrayList<>())
+ .add(new T2(entry.getKey(), entry.getValue().get2().values().stream().min((cp1, cp2) ->
+ Long.compare(cp1.timestamp(), cp2.timestamp())).get()));
+ }
+ }
+
+ if (!F.isEmpty(notReservedCachesToPrint)) {
+ log.info("Following caches were not reserved [" +
Review comment:
Following caches were not reserved -> Caches groups were not reserved
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org