hemantk-12 commented on code in PR #8124:
URL: https://github.com/apache/ozone/pull/8124#discussion_r2036226024
##########
hadoop-ozone/cli-shell/src/main/java/org/apache/hadoop/ozone/shell/snapshot/ListSnapshotDiffHandler.java:
##########
@@ -37,38 +38,32 @@ public class ListSnapshotDiffHandler extends Handler {
@CommandLine.Mixin
private BucketUri snapshotPath;
- @CommandLine.Option(names = {"-s", "--status"},
+ @CommandLine.Option(names = {"--job-status"},
description = "List jobs based on status.\n" +
"Accepted values are: queued, in_progress, done, failed, rejected",
defaultValue = "in_progress")
private String jobStatus;
- @CommandLine.Option(names = {"-a", "--all"},
- description = "List all jobs regardless of status.",
- defaultValue = "false")
- private boolean listAll;
+ @CommandLine.Mixin
+ private ListOptions listOptions;
@Override
protected OzoneAddress getAddress() {
return snapshotPath.getValue();
}
@Override
- protected void execute(OzoneClient client, OzoneAddress address)
- throws IOException {
-
+ protected void execute(OzoneClient client, OzoneAddress address) throws
IOException {
Review Comment:
Done.
##########
hadoop-ozone/cli-shell/src/main/java/org/apache/hadoop/ozone/shell/snapshot/ListSnapshotDiffHandler.java:
##########
@@ -37,38 +38,32 @@ public class ListSnapshotDiffHandler extends Handler {
@CommandLine.Mixin
private BucketUri snapshotPath;
- @CommandLine.Option(names = {"-s", "--status"},
+ @CommandLine.Option(names = {"--job-status"},
description = "List jobs based on status.\n" +
"Accepted values are: queued, in_progress, done, failed, rejected",
defaultValue = "in_progress")
private String jobStatus;
- @CommandLine.Option(names = {"-a", "--all"},
- description = "List all jobs regardless of status.",
- defaultValue = "false")
- private boolean listAll;
+ @CommandLine.Mixin
+ private ListOptions listOptions;
@Override
protected OzoneAddress getAddress() {
return snapshotPath.getValue();
}
@Override
- protected void execute(OzoneClient client, OzoneAddress address)
- throws IOException {
-
+ protected void execute(OzoneClient client, OzoneAddress address) throws
IOException {
String volumeName = snapshotPath.getValue().getVolumeName();
String bucketName = snapshotPath.getValue().getBucketName();
- List<OzoneSnapshotDiff> jobList =
- client.getObjectStore().listSnapshotDiffJobs(
- volumeName, bucketName, jobStatus, listAll);
+ Iterator<OzoneSnapshotDiff> iterator = client.getObjectStore()
+ .listSnapshotDiffJobs(volumeName, bucketName, jobStatus,
listOptions.isAll(), listOptions.getStartItem());
+
+ int counter = printAsJsonArray(iterator, listOptions.getLimit());
Review Comment:
Sorry, I didn't quite follow it. In case `all` is passed, we don't know how
many entries there are, so we print all the maximum possible values and it is
used only for the handler.
For the server call, we use
[listCacheSize](https://github.com/apache/ozone/blob/84d14a32050b5d4b3a202087d21f92b0b3a03cb0/hadoop-ozone/client/src/main/java/org/apache/hadoop/ozone/client/ObjectStore.java#L801)
no matter what is set in listOptions.
##########
hadoop-ozone/interface-client/src/main/proto/OmClientProtocol.proto:
##########
@@ -1980,6 +1980,8 @@ message ListSnapshotDiffJobRequest {
required string bucketName = 2;
optional string jobStatus = 3;
optional bool listAll = 4;
+ optional string prevSnapshotDiffJob = 5;
+ optional uint32 maxListResult = 6;
Review Comment:
First, this API is mainly for operational purpose, so I wonder if anyone
would use it other than the CLI. Backward is broken in the sense that it will
not return more than 1000 entries in case `maxListResult` is not passed or it
is `0` (changed in
https://github.com/apache/ozone/pull/8124/commits/9491f36e7fc8f5316520a250f75c38624c0ddc9c#diff-857b24769ac9c1c67d015bdffc1227dc2b4751de3790b9fd89269a3c46a5218aR836).
##########
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/snapshot/SnapshotDiffManager.java:
##########
@@ -422,30 +423,45 @@ public CancelSnapshotDiffResponse cancelSnapshotDiff(
return new CancelSnapshotDiffResponse(reason);
}
- public List<SnapshotDiffJob> getSnapshotDiffJobList(
- String volumeName, String bucketName,
- String jobStatus, boolean listAll) throws IOException {
- List<SnapshotDiffJob> jobList = new ArrayList<>();
+ public ListSnapshotDiffJobResponse getSnapshotDiffJobList(
+ String volumeName,
+ String bucketName,
+ String jobStatus,
+ boolean listAll,
+ String prevDiffJob,
+ int maxEntries) throws IOException {
+ List<SnapshotDiffJob> jobs = new ArrayList<>();
+ String lastSnapshotDiffJob = null;
try (ClosableIterator<Map.Entry<String, SnapshotDiffJob>> iterator =
- snapDiffJobTable.iterator()) {
- while (iterator.hasNext()) {
- SnapshotDiffJob snapshotDiffJob = iterator.next().getValue();
+ snapDiffJobTable.iterator(Optional.ofNullable(prevDiffJob),
Optional.empty())) {
Review Comment:
It is hard to validate the `prevDiffJob`. What if the value is deleted by
the time the request is made?
--
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]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
