You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@geode.apache.org by GitBox <gi...@apache.org> on 2020/06/23 23:33:54 UTC
[GitHub] [geode] DonalEvans commented on a change in pull request #5281: First clean run of Rest Command for Restore Redundancy
DonalEvans commented on a change in pull request #5281:
URL: https://github.com/apache/geode/pull/5281#discussion_r444556037
##########
File path: geode-core/src/main/java/org/apache/geode/internal/cache/control/SerializableRestoreRedundancyResultsImpl.java
##########
@@ -39,7 +39,8 @@
public void addPrimaryReassignmentDetails(PartitionRebalanceInfo details) {
totalPrimaryTransfersCompleted += details.getPrimaryTransfersCompleted();
totalPrimaryTransferTime =
- totalPrimaryTransferTime.plusMillis(details.getPrimaryTransferTime());
+ totalPrimaryTransferTime + details.getPrimaryTransferTime();
Review comment:
This could be replaced with `totalPrimaryTransferTime += details.getPrimaryTransferTime();`
##########
File path: geode-management/src/main/java/org/apache/geode/management/internal/operation/RestoreRedundancyResultsImpl.java
##########
@@ -38,24 +37,55 @@
public static final String PRIMARY_TRANSFER_TIME = "Total primary transfer time (ms) = ";
private static final long serialVersionUID = -1174735246756963521L;
- protected Map<String, RegionRedundancyStatus> zeroRedundancyRegions = new HashMap<>();
- protected Map<String, RegionRedundancyStatus> underRedundancyRegions = new HashMap<>();
- protected Map<String, RegionRedundancyStatus> satisfiedRedundancyRegions = new HashMap<>();
+ protected Map<String, RegionRedundancyStatus> zeroRedundancyRegionsResults = new HashMap<>();
+ protected Map<String, RegionRedundancyStatus> underRedundancyRegionsResults = new HashMap<>();
+ protected Map<String, RegionRedundancyStatus> satisfiedRedundancyRegionsResults = new HashMap<>();
protected int totalPrimaryTransfersCompleted;
- protected Duration totalPrimaryTransferTime = Duration.ZERO;
+ protected long totalPrimaryTransferTime = 0;
protected boolean success = true;
protected String statusMessage;
protected final List<String> includedRegionsWithNoMembers = new ArrayList<>();
+ private RegionRedundancyStatus regionResult;
+
+ public void setZeroRedundancyRegionsResults(
+ Map<String, RegionRedundancyStatus> zeroRedundancyRegionsResults) {
+ this.zeroRedundancyRegionsResults = zeroRedundancyRegionsResults;
+ }
+
+ public void setUnderRedundancyRegionsResults(
+ Map<String, RegionRedundancyStatus> underRedundancyRegionsResults) {
+ this.underRedundancyRegionsResults = underRedundancyRegionsResults;
+ }
+
+ public void setSatisfiedRedundancyRegionsResults(
+ Map<String, RegionRedundancyStatus> satisfiedRedundancyRegionsResults) {
+ this.satisfiedRedundancyRegionsResults = satisfiedRedundancyRegionsResults;
+ }
+
+ public void setTotalPrimaryTransfersCompleted(int totalPrimaryTransfersCompleted) {
+ this.totalPrimaryTransfersCompleted = totalPrimaryTransfersCompleted;
+ }
+
+ public void setTotalPrimaryTransferTime(long totalPrimaryTransferTime) {
+ this.totalPrimaryTransferTime = totalPrimaryTransferTime;
+ }
+
+ public void setRegionResult(RegionRedundancyStatus regionResult) {
+ this.regionResult = regionResult;
+ }
+
+
+ public RestoreRedundancyResultsImpl() {}
public void addRegionResults(RestoreRedundancyResults results) {
- satisfiedRedundancyRegions.putAll(results.getSatisfiedRedundancyRegionResults());
- underRedundancyRegions.putAll(results.getUnderRedundancyRegionResults());
- zeroRedundancyRegions.putAll(results.getZeroRedundancyRegionResults());
+ satisfiedRedundancyRegionsResults.putAll(results.getSatisfiedRedundancyRegionResults());
+ underRedundancyRegionsResults.putAll(results.getUnderRedundancyRegionResults());
+ zeroRedundancyRegionsResults.putAll(results.getZeroRedundancyRegionResults());
totalPrimaryTransfersCompleted += results.getTotalPrimaryTransfersCompleted();
totalPrimaryTransferTime =
- totalPrimaryTransferTime.plus(results.getTotalPrimaryTransferTime());
+ totalPrimaryTransferTime + results.getTotalPrimaryTransferTime();
Review comment:
This can be replaced with `totalPrimaryTransferTime += results.getTotalPrimaryTransferTime();`
##########
File path: geode-core/src/test/java/org/apache/geode/internal/cache/control/SerializableRestoreRedundancyResultsImplTest.java
##########
@@ -153,39 +157,64 @@ public void addRegionResultsAddsToCorrectInternalMapAndAddsPrimaryReassignmentDe
when(regionResults.getSatisfiedRedundancyRegionResults())
.thenReturn(Collections.singletonMap(successfulRegionName, successfulRegionResult));
when(regionResults.getTotalPrimaryTransfersCompleted()).thenReturn(transfersCompleted);
- when(regionResults.getTotalPrimaryTransferTime()).thenReturn(Duration.ofMillis(transferTime));
+ when(regionResults.getTotalPrimaryTransferTime()).thenReturn(transferTime);
results.addRegionResults(regionResults);
Map<String, RegionRedundancyStatus> zeroRedundancyResults =
results.getZeroRedundancyRegionResults();
- assertThat(zeroRedundancyResults.size(), is(1));
- assertThat(zeroRedundancyResults.get(zeroRedundancyRegionName), is(zeroRedundancyRegionResult));
+ assertThat(zeroRedundancyResults.size()).isEqualTo(1);
+ assertThat(zeroRedundancyResults.get(zeroRedundancyRegionName))
+ .isEqualTo(zeroRedundancyRegionResult);
Map<String, RegionRedundancyStatus> underRedundancyResults =
results.getUnderRedundancyRegionResults();
- assertThat(underRedundancyResults.size(), is(1));
- assertThat(underRedundancyResults.get(underRedundancyRegionName),
- is(underRedundancyRegionResult));
+ assertThat(underRedundancyResults.size()).isEqualTo(1);
+ assertThat(underRedundancyResults.get(underRedundancyRegionName))
+ .isEqualTo(underRedundancyRegionResult);
Map<String, RegionRedundancyStatus> successfulRegionResults =
results.getSatisfiedRedundancyRegionResults();
- assertThat(successfulRegionResults.size(), is(1));
- assertThat(successfulRegionResults.get(successfulRegionName), is(successfulRegionResult));
+ assertThat(successfulRegionResults.size()).isEqualTo(1);
+ assertThat(successfulRegionResults.get(successfulRegionName)).isEqualTo(successfulRegionResult);
- assertThat(results.getTotalPrimaryTransfersCompleted(), is(transfersCompleted));
- assertThat(results.getTotalPrimaryTransferTime().toMillis(), is(transferTime));
+ assertThat(results.getTotalPrimaryTransfersCompleted()).isEqualTo(transfersCompleted);
+ assertThat(results.getTotalPrimaryTransferTime()).isEqualTo(transferTime);
}
@Test
public void addPrimaryDetailsUpdatesValue() {
- assertThat(results.getTotalPrimaryTransfersCompleted(), is(0));
- assertThat(results.getTotalPrimaryTransferTime().toMillis(), is(0L));
+ assertThat(results.getTotalPrimaryTransfersCompleted()).isEqualTo(0);
+ assertThat(results.getTotalPrimaryTransferTime()).isEqualTo(0L);
results.addPrimaryReassignmentDetails(details);
- assertThat(results.getTotalPrimaryTransfersCompleted(), is(transfersCompleted));
- assertThat(results.getTotalPrimaryTransferTime().toMillis(), is(transferTime));
+ assertThat(results.getTotalPrimaryTransfersCompleted()).isEqualTo(transfersCompleted);
+ assertThat(results.getTotalPrimaryTransferTime()).isEqualTo(transferTime);
results.addPrimaryReassignmentDetails(details);
- assertThat(results.getTotalPrimaryTransfersCompleted(), is(transfersCompleted * 2));
- assertThat(results.getTotalPrimaryTransferTime().toMillis(), is(transferTime * 2));
+ assertThat(results.getTotalPrimaryTransfersCompleted()).isEqualTo(transfersCompleted * 2);
+ assertThat(results.getTotalPrimaryTransferTime()).isEqualTo(transferTime * 2);
+ }
+
+ @Test
+ public void testSerializable() throws JsonProcessingException {
Review comment:
This test sets multiple fields but does not verify all of them. Would it be possible to confirm that all data in the object is correctly serialized and deserialized, not just the message and status fields?
----------------------------------------------------------------
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