Skip to content

Commit

Permalink
Addressing comments
Browse files Browse the repository at this point in the history 
Signed-off-by: Shourya Dutta Biswas <[email protected]>
  • Loading branch information
@shourya035
shourya035 committed Aug 1, 2023
1 parent 6ce96a8 commit 508eb22
Show file tree
Hide file tree
Showing 7 changed files with 230 additions and 234 deletions.
71 changes: 35 additions & 36 deletions server/src/internalClusterTest/java/org/opensearch/remotestore/RemoteStoreStatsIT.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,7 @@ public void testStatsResponseFromAllNodes() {
assertEquals(1, matches.size());
RemoteSegmentTransferTracker.Stats stats = matches.get(0).getStats();
validateUploadStats(stats);
assertEquals(0, stats.directoryFileTransferTrackerStats.downloadBytesStarted);
assertEquals(0, stats.directoryFileTransferTrackerStats.transferredBytesStarted);
}

// Step 3 - Enable replicas on the existing indices and ensure that download
Expand All @@ -93,7 +93,7 @@ public void testStatsResponseFromAllNodes() {
RemoteSegmentTransferTracker.Stats stats = stat.getStats();
if (routing.primary()) {
validateUploadStats(stats);
assertEquals(0, stats.directoryFileTransferTrackerStats.downloadBytesStarted);
assertEquals(0, stats.directoryFileTransferTrackerStats.transferredBytesStarted);
} else {
validateDownloadStats(stats);
assertEquals(0, stats.totalUploadsStarted);
Expand Down Expand Up @@ -124,7 +124,7 @@ public void testStatsResponseAllShards() {
assertTrue(response.getRemoteStoreStats() != null && response.getRemoteStoreStats().length == 3);
RemoteSegmentTransferTracker.Stats stats = response.getRemoteStoreStats()[0].getStats();
validateUploadStats(stats);
assertEquals(0, stats.directoryFileTransferTrackerStats.downloadBytesStarted);
assertEquals(0, stats.directoryFileTransferTrackerStats.transferredBytesStarted);

// Step 3 - Enable replicas on the existing indices and ensure that download
// stats are being populated as well
Expand All @@ -138,7 +138,7 @@ public void testStatsResponseAllShards() {
stats = stat.getStats();
if (routing.primary()) {
validateUploadStats(stats);
assertEquals(0, stats.directoryFileTransferTrackerStats.downloadBytesStarted);
assertEquals(0, stats.directoryFileTransferTrackerStats.transferredBytesStarted);
} else {
validateDownloadStats(stats);
assertEquals(0, stats.totalUploadsStarted);
Expand Down Expand Up @@ -171,7 +171,7 @@ public void testStatsResponseFromLocalNode() {
assertTrue(response.getRemoteStoreStats() != null && response.getRemoteStoreStats().length == 1);
RemoteSegmentTransferTracker.Stats stats = response.getRemoteStoreStats()[0].getStats();
validateUploadStats(stats);
assertEquals(0, stats.directoryFileTransferTrackerStats.downloadBytesStarted);
assertEquals(0, stats.directoryFileTransferTrackerStats.transferredBytesStarted);
}
changeReplicaCountAndEnsureGreen(1);
for (String node : nodes) {
Expand All @@ -188,7 +188,7 @@ public void testStatsResponseFromLocalNode() {
RemoteSegmentTransferTracker.Stats stats = stat.getStats();
if (routing.primary()) {
validateUploadStats(stats);
assertEquals(0, stats.directoryFileTransferTrackerStats.downloadBytesStarted);
assertEquals(0, stats.directoryFileTransferTrackerStats.transferredBytesStarted);
} else {
validateDownloadStats(stats);
assertEquals(0, stats.totalUploadsStarted);
Expand Down Expand Up @@ -241,8 +241,8 @@ public void testDownloadStatsCorrectnessSinglePrimarySingleReplica() throws Exce
.get(0)
.getStats();
assertTrue(
zeroStateReplicaStats.directoryFileTransferTrackerStats.downloadBytesStarted == 0
&& zeroStateReplicaStats.directoryFileTransferTrackerStats.downloadBytesSucceeded == 0
zeroStateReplicaStats.directoryFileTransferTrackerStats.transferredBytesStarted == 0
&& zeroStateReplicaStats.directoryFileTransferTrackerStats.transferredBytesSucceeded == 0
);

// Index documents
Expand Down Expand Up @@ -270,18 +270,18 @@ public void testDownloadStatsCorrectnessSinglePrimarySingleReplica() throws Exce
assertTrue(primaryStats.totalUploadsStarted > 0);
assertTrue(primaryStats.totalUploadsSucceeded > 0);
assertTrue(
replicaStats.directoryFileTransferTrackerStats.downloadBytesStarted > 0
replicaStats.directoryFileTransferTrackerStats.transferredBytesStarted > 0
&& primaryStats.uploadBytesStarted
- zeroStatePrimaryStats.uploadBytesStarted == replicaStats.directoryFileTransferTrackerStats.downloadBytesStarted
- zeroStatePrimaryStats.uploadBytesStarted == replicaStats.directoryFileTransferTrackerStats.transferredBytesStarted
);
assertTrue(
replicaStats.directoryFileTransferTrackerStats.downloadBytesSucceeded > 0
replicaStats.directoryFileTransferTrackerStats.transferredBytesSucceeded > 0
&& primaryStats.uploadBytesSucceeded
- zeroStatePrimaryStats.uploadBytesSucceeded == replicaStats.directoryFileTransferTrackerStats.downloadBytesSucceeded
- zeroStatePrimaryStats.uploadBytesSucceeded == replicaStats.directoryFileTransferTrackerStats.transferredBytesSucceeded
);
// Assert zero failures
assertEquals(0, primaryStats.uploadBytesFailed);
assertEquals(0, replicaStats.directoryFileTransferTrackerStats.downloadBytesFailed);
assertEquals(0, replicaStats.directoryFileTransferTrackerStats.transferredBytesFailed);
}, 60, TimeUnit.SECONDS);
}
}
Expand Down Expand Up @@ -333,8 +333,8 @@ public void testDownloadStatsCorrectnessSinglePrimaryMultipleReplicaShards() thr
.collect(Collectors.toList());
zeroStateReplicaStats.forEach(stats -> {
assertTrue(
stats.getStats().directoryFileTransferTrackerStats.downloadBytesStarted == 0
&& stats.getStats().directoryFileTransferTrackerStats.downloadBytesSucceeded == 0
stats.getStats().directoryFileTransferTrackerStats.transferredBytesStarted == 0
&& stats.getStats().directoryFileTransferTrackerStats.transferredBytesSucceeded == 0
);
});

Expand All @@ -360,9 +360,9 @@ public void testDownloadStatsCorrectnessSinglePrimaryMultipleReplicaShards() thr
uploadBytesSucceeded = stats.uploadBytesSucceeded;
uploadBytesFailed = stats.uploadBytesFailed;
} else {
downloadBytesStarted.add(stats.directoryFileTransferTrackerStats.downloadBytesStarted);
downloadBytesSucceeded.add(stats.directoryFileTransferTrackerStats.downloadBytesSucceeded);
downloadBytesFailed.add(stats.directoryFileTransferTrackerStats.downloadBytesFailed);
downloadBytesStarted.add(stats.directoryFileTransferTrackerStats.transferredBytesStarted);
downloadBytesSucceeded.add(stats.directoryFileTransferTrackerStats.transferredBytesSucceeded);
downloadBytesFailed.add(stats.directoryFileTransferTrackerStats.transferredBytesFailed);
}
}

Expand Down Expand Up @@ -469,28 +469,27 @@ public void testStatsOnRemoteStoreRestore() throws IOException {
internalCluster().startDataOnlyNode();

// Restore index from remote store
client().admin().cluster().restoreRemoteStore(new RestoreRemoteStoreRequest().indices(INDEX_NAME), PlainActionFuture.newFuture());
assertAcked(client().admin().indices().prepareClose(INDEX_NAME));
client().admin()
.cluster()
.restoreRemoteStore(new RestoreRemoteStoreRequest().indices(INDEX_NAME).restoreAllShards(true), PlainActionFuture.newFuture());

// Ensure that the index is green
ensureGreen(INDEX_NAME);

// Index some more docs to force segment uploads to remote store
indexDocs();

RemoteStoreStats[] remoteStoreStats = client().admin()
.cluster()
.prepareRemoteStoreStats(INDEX_NAME, "0")
.get()
.getRemoteStoreStats();
Arrays.stream(remoteStoreStats).forEach(statObject -> {
RemoteStoreStatsResponse remoteStoreStatsResponse = client().admin().cluster().prepareRemoteStoreStats(INDEX_NAME, "0").get();
Arrays.stream(remoteStoreStatsResponse.getRemoteStoreStats()).forEach(statObject -> {
RemoteSegmentTransferTracker.Stats segmentTracker = statObject.getStats();
// Assert that we have both upload and download stats for the index
assertTrue(
segmentTracker.totalUploadsStarted > 0 && segmentTracker.totalUploadsSucceeded > 0 && segmentTracker.totalUploadsFailed == 0
);
assertTrue(
segmentTracker.directoryFileTransferTrackerStats.downloadBytesStarted > 0
&& segmentTracker.directoryFileTransferTrackerStats.downloadBytesSucceeded > 0
segmentTracker.directoryFileTransferTrackerStats.transferredBytesStarted > 0
&& segmentTracker.directoryFileTransferTrackerStats.transferredBytesSucceeded > 0
);
});
}
Expand Down Expand Up @@ -521,8 +520,8 @@ public void testNonZeroPrimaryStatsOnNewlyCreatedIndexWithZeroDocs() throws Exce
);
} else {
assertTrue(
segmentTracker.directoryFileTransferTrackerStats.downloadBytesStarted == 0
&& segmentTracker.directoryFileTransferTrackerStats.downloadBytesSucceeded == 0
segmentTracker.directoryFileTransferTrackerStats.transferredBytesStarted == 0
&& segmentTracker.directoryFileTransferTrackerStats.transferredBytesSucceeded == 0
);
}
});
Expand Down Expand Up @@ -577,13 +576,13 @@ private void validateUploadStats(RemoteSegmentTransferTracker.Stats stats) {
}

private void validateDownloadStats(RemoteSegmentTransferTracker.Stats stats) {
assertTrue(stats.directoryFileTransferTrackerStats.lastDownloadTimestampMs > 0);
assertTrue(stats.directoryFileTransferTrackerStats.downloadBytesStarted > 0);
assertTrue(stats.directoryFileTransferTrackerStats.downloadBytesSucceeded > 0);
assertEquals(stats.directoryFileTransferTrackerStats.downloadBytesFailed, 0);
assertTrue(stats.directoryFileTransferTrackerStats.lastSuccessfulSegmentDownloadBytes > 0);
assertTrue(stats.directoryFileTransferTrackerStats.downloadBytesMovingAverage > 0);
assertTrue(stats.directoryFileTransferTrackerStats.downloadBytesPerSecMovingAverage > 0);
assertTrue(stats.directoryFileTransferTrackerStats.lastTransferTimestampMs > 0);
assertTrue(stats.directoryFileTransferTrackerStats.transferredBytesStarted > 0);
assertTrue(stats.directoryFileTransferTrackerStats.transferredBytesSucceeded > 0);
assertEquals(stats.directoryFileTransferTrackerStats.transferredBytesFailed, 0);
assertTrue(stats.directoryFileTransferTrackerStats.lastSuccessfulTransferInBytes > 0);
assertTrue(stats.directoryFileTransferTrackerStats.transferredBytesMovingAverage > 0);
assertTrue(stats.directoryFileTransferTrackerStats.transferredBytesPerSecMovingAverage > 0);
}

// Validate if the shardRouting obtained from cluster state contains the exact same routing object
Expand Down
16 changes: 8 additions & 8 deletions ...src/main/java/org/opensearch/action/admin/cluster/remotestore/stats/RemoteStoreStats.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@ public XContentBuilder toXContent(XContentBuilder builder, Params params) throws
builder.startObject(Fields.SEGMENT);
builder.startObject(SubFields.DOWNLOAD);
// Ensuring that we are not showing 0 metrics to the user
if (remoteSegmentShardStats.directoryFileTransferTrackerStats.downloadBytesStarted != 0) {
if (remoteSegmentShardStats.directoryFileTransferTrackerStats.transferredBytesStarted != 0) {
buildDownloadStats(builder);
}
builder.endObject();
Expand Down Expand Up @@ -107,19 +107,19 @@ private void buildUploadStats(XContentBuilder builder) throws IOException {
private void buildDownloadStats(XContentBuilder builder) throws IOException {
builder.field(
DownloadStatsFields.LAST_SYNC_TIMESTAMP,
remoteSegmentShardStats.directoryFileTransferTrackerStats.lastDownloadTimestampMs
remoteSegmentShardStats.directoryFileTransferTrackerStats.lastTransferTimestampMs
);
builder.startObject(DownloadStatsFields.TOTAL_DOWNLOADS_IN_BYTES)
.field(SubFields.STARTED, remoteSegmentShardStats.directoryFileTransferTrackerStats.downloadBytesStarted)
.field(SubFields.SUCCEEDED, remoteSegmentShardStats.directoryFileTransferTrackerStats.downloadBytesSucceeded)
.field(SubFields.FAILED, remoteSegmentShardStats.directoryFileTransferTrackerStats.downloadBytesFailed);
.field(SubFields.STARTED, remoteSegmentShardStats.directoryFileTransferTrackerStats.transferredBytesStarted)
.field(SubFields.SUCCEEDED, remoteSegmentShardStats.directoryFileTransferTrackerStats.transferredBytesSucceeded)
.field(SubFields.FAILED, remoteSegmentShardStats.directoryFileTransferTrackerStats.transferredBytesFailed);
builder.endObject();
builder.startObject(DownloadStatsFields.DOWNLOAD_SIZE_IN_BYTES)
.field(SubFields.LAST_SUCCESSFUL, remoteSegmentShardStats.directoryFileTransferTrackerStats.lastSuccessfulSegmentDownloadBytes)
.field(SubFields.MOVING_AVG, remoteSegmentShardStats.directoryFileTransferTrackerStats.downloadBytesMovingAverage);
.field(SubFields.LAST_SUCCESSFUL, remoteSegmentShardStats.directoryFileTransferTrackerStats.lastSuccessfulTransferInBytes)
.field(SubFields.MOVING_AVG, remoteSegmentShardStats.directoryFileTransferTrackerStats.transferredBytesMovingAverage);
builder.endObject();
builder.startObject(DownloadStatsFields.DOWNLOAD_SPEED_IN_BYTES_PER_SEC)
.field(SubFields.MOVING_AVG, remoteSegmentShardStats.directoryFileTransferTrackerStats.downloadBytesPerSecMovingAverage);
.field(SubFields.MOVING_AVG, remoteSegmentShardStats.directoryFileTransferTrackerStats.transferredBytesPerSecMovingAverage);
builder.endObject();
}

Expand Down
Loading

0 comments on commit 508eb22

Please sign in to comment.