From 9602e2fd36edcf3a838b868124a940a01e2b50f2 Mon Sep 17 00:00:00 2001 From: Hemant Kumar Date: Wed, 29 Nov 2023 10:10:58 -0800 Subject: [PATCH 1/4] Throw an IOException is continuation token for snapshot diff is more than the total diff entries --- .../ozone/snapshot/SnapshotDiffReportOzone.java | 17 ++++++++++------- .../ozone/om/snapshot/TestOmSnapshot.java | 12 ++++++++++++ .../ozone/om/snapshot/SnapshotDiffManager.java | 11 ++++++----- 3 files changed, 28 insertions(+), 12 deletions(-) diff --git a/hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/snapshot/SnapshotDiffReportOzone.java b/hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/snapshot/SnapshotDiffReportOzone.java index aec0c6d12ccd..aa145807eaba 100644 --- a/hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/snapshot/SnapshotDiffReportOzone.java +++ b/hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/snapshot/SnapshotDiffReportOzone.java @@ -101,13 +101,16 @@ public String toString() { .append(" and snapshot: ") .append(getLaterSnapshotName()) .append(LINE_SEPARATOR); - for (DiffReportEntry entry : getDiffList()) { - str.append(entry.toString()).append(LINE_SEPARATOR); - } - if (StringUtils.isNotEmpty(token)) { - str.append("Next token: ") - .append(token) - .append(LINE_SEPARATOR); + if (!getDiffList().isEmpty()) { + for (DiffReportEntry entry : getDiffList()) { + str.append(entry.toString()).append(LINE_SEPARATOR); + } + if (StringUtils.isNotEmpty(token)) { + str.append("Next token: ") + .append(token); + } + } else { + str.append("No diff or no more diff for with the request parameters."); } return str.toString(); } diff --git a/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/om/snapshot/TestOmSnapshot.java b/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/om/snapshot/TestOmSnapshot.java index ff444a88cebe..d5da25e718ae 100644 --- a/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/om/snapshot/TestOmSnapshot.java +++ b/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/om/snapshot/TestOmSnapshot.java @@ -1429,6 +1429,18 @@ public void testSnapDiff() throws Exception { SnapshotDiffReport.DiffType.MODIFY, key3) ); assertEquals(expectedDiffList, diff5.getDiffList()); + + SnapshotDiffReportOzone diff12 = store.snapshotDiff(volume, bucket, snap6, + snap7, null, 2, forceFullSnapshotDiff, disableNativeDiff) + .getSnapshotDiffReport(); + + IOException ioException = assertThrows(IOException.class, + () -> store.snapshotDiff(volume, bucket, snap6, + snap7, "2", 0, forceFullSnapshotDiff, disableNativeDiff)); + assertThat(ioException.getMessage(), containsString("Index (given: 2) " + + "should be a number >= 0 and < totalDiffEntries: 2. Page size " + + "(given: 2) should be a positive number > 0.")); + } @Test diff --git a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/snapshot/SnapshotDiffManager.java b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/snapshot/SnapshotDiffManager.java index 651ed06cbe1c..c34594cffe88 100644 --- a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/snapshot/SnapshotDiffManager.java +++ b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/snapshot/SnapshotDiffManager.java @@ -569,11 +569,12 @@ SnapshotDiffReportOzone createPageResponse( final int index, final int pageSize ) throws IOException { - if (index < 0 || pageSize <= 0) { - throw new IllegalArgumentException(String.format( - "Index should be a number >= 0. Given index %d. Page size " + - "should be a positive number > 0. Given page size is %d", - index, pageSize)); + if (index < 0 || index > snapDiffJob.getTotalDiffEntries() + || pageSize <= 0) { + throw new IOException(String.format( + "Index (given: %d) should be a number >= 0 and < totalDiffEntries: " + + "%d. Page size (given: %d) should be a positive number > 0.", + index, snapDiffJob.getTotalDiffEntries(), pageSize)); } OFSPath path = getSnapshotRootPath(volumeName, bucketName); From 9bb382351735da8317917c015e9d98338baa8271 Mon Sep 17 00:00:00 2001 From: Hemant Kumar Date: Tue, 28 Nov 2023 22:39:23 -0800 Subject: [PATCH 2/4] Fixed findbugs --- .../org/apache/hadoop/ozone/om/snapshot/TestOmSnapshot.java | 4 ---- 1 file changed, 4 deletions(-) diff --git a/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/om/snapshot/TestOmSnapshot.java b/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/om/snapshot/TestOmSnapshot.java index d5da25e718ae..3b42b2f4c945 100644 --- a/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/om/snapshot/TestOmSnapshot.java +++ b/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/om/snapshot/TestOmSnapshot.java @@ -1430,10 +1430,6 @@ public void testSnapDiff() throws Exception { ); assertEquals(expectedDiffList, diff5.getDiffList()); - SnapshotDiffReportOzone diff12 = store.snapshotDiff(volume, bucket, snap6, - snap7, null, 2, forceFullSnapshotDiff, disableNativeDiff) - .getSnapshotDiffReport(); - IOException ioException = assertThrows(IOException.class, () -> store.snapshotDiff(volume, bucket, snap6, snap7, "2", 0, forceFullSnapshotDiff, disableNativeDiff)); From 8d2a1d7ded5e1e36b1fe910ecb82c25fcb71b616 Mon Sep 17 00:00:00 2001 From: Hemant Kumar Date: Wed, 29 Nov 2023 09:19:03 -0800 Subject: [PATCH 3/4] Fixed tests --- .../org/apache/hadoop/ozone/om/snapshot/TestOmSnapshot.java | 6 +++--- .../hadoop/ozone/om/snapshot/TestSnapshotDiffManager.java | 4 ++-- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/om/snapshot/TestOmSnapshot.java b/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/om/snapshot/TestOmSnapshot.java index 3b42b2f4c945..7e9fe787df67 100644 --- a/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/om/snapshot/TestOmSnapshot.java +++ b/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/om/snapshot/TestOmSnapshot.java @@ -1432,10 +1432,10 @@ public void testSnapDiff() throws Exception { IOException ioException = assertThrows(IOException.class, () -> store.snapshotDiff(volume, bucket, snap6, - snap7, "2", 0, forceFullSnapshotDiff, disableNativeDiff)); - assertThat(ioException.getMessage(), containsString("Index (given: 2) " + + snap7, "3", 0, forceFullSnapshotDiff, disableNativeDiff)); + assertThat(ioException.getMessage(), containsString("Index (given: 3) " + "should be a number >= 0 and < totalDiffEntries: 2. Page size " + - "(given: 2) should be a positive number > 0.")); + "(given: 1000) should be a positive number > 0.")); } diff --git a/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/snapshot/TestSnapshotDiffManager.java b/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/snapshot/TestSnapshotDiffManager.java index 8d8921ddff81..41b6f73d8cb0 100644 --- a/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/snapshot/TestSnapshotDiffManager.java +++ b/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/snapshot/TestSnapshotDiffManager.java @@ -893,7 +893,7 @@ private DiffReportEntry getTestDiffEntry(String jobId, * objectId Map of diff keys to be checked with their corresponding key names. */ @ParameterizedTest - @CsvSource({"0,10,1000", "1,10,8", "1000,1000,10", "-1,1000,10000", + @CsvSource({"0,10,1000", "1,10,8", "10,1000,10", "-1,1000,10000", "1,0,1000", "1,-1,1000"}) public void testCreatePageResponse(int startIdx, int pageSize, @@ -933,7 +933,7 @@ public void testCreatePageResponse(int startIdx, codecRegistry.asRawData(snapshotDiffJob2)); if (pageSize <= 0 || startIdx < 0) { - Assertions.assertThrows(IllegalArgumentException.class, + Assertions.assertThrows(IOException.class, () -> snapshotDiffManager.createPageResponse(snapshotDiffJob, "vol", "buck", "fs", "ts", startIdx, pageSize)); return; From 137726a0fdf590d56cdfad4b6b43a93935ceef24 Mon Sep 17 00:00:00 2001 From: Hemant Kumar Date: Wed, 29 Nov 2023 11:30:30 -0800 Subject: [PATCH 4/4] Fixed the message --- .../apache/hadoop/ozone/snapshot/SnapshotDiffReportOzone.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/snapshot/SnapshotDiffReportOzone.java b/hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/snapshot/SnapshotDiffReportOzone.java index aa145807eaba..3d14e266daa4 100644 --- a/hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/snapshot/SnapshotDiffReportOzone.java +++ b/hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/snapshot/SnapshotDiffReportOzone.java @@ -110,7 +110,7 @@ public String toString() { .append(token); } } else { - str.append("No diff or no more diff for with the request parameters."); + str.append("No diff or no more diff for the request parameters."); } return str.toString(); }