You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@ozone.apache.org by GitBox <gi...@apache.org> on 2021/04/06 08:34:10 UTC
[GitHub] [ozone] neils-dev opened a new pull request #2110: HDDS-4043. allow deletion from Trash directory without -skipTrash option
neils-dev opened a new pull request #2110:
URL: https://github.com/apache/ozone/pull/2110
## What changes were proposed in this pull request?
To have FsShell _**rm**_ (eg. `bin/ozone fs -rm o3fs://bucket.volume/key`) commands delete items from the filesystem Trash **_without_** need of the _-skipTrash_ flag, changes are made to the ozone filesystem specific TrashPolicy. Implemented through overriding the TrashPolicy _moveTrash_ method for TrashPolicyOzone.
Here if the item to be deleted with the FsShell _**rm**_ command is located in the key root trash directory, it is deleted from the filesystem. Before this patch, items removed through the rm command were moved to the root Trash directory when trash was enabled. Patch enacts similar behavior as that of FsShell **_rm_** command with the -_skipTrash_ flag specified. Behavior with the _-skipTrash_ flag enabled is unaffected.
Necessary for this patch is configuring the hadoop filesystem to use the ozone implementation specific **_TrashPolicyOzone_** though the '_core-site.xml_' as in :
`<property>
<name>fs.trash.classname</name>
<value>org.apache.hadoop.ozone.om.TrashPolicyOzone</value>
</property>`
## What is the link to the Apache JIRA
https://issues.apache.org/jira/browse/HDDS-4043
## How was this patch tested?
Patch was tested through both integration tests and manual tests.
**Integration test** implemented in ozone shell integration test : TestOzoneShellHA.java. Tested through `hadoop-ozone/integration_test$ mvn -Dtest=TestOzoneShellHA#testDeleteTrashNoSkipTrash test`
-------------------------------------------------------------------------------
Test set: org.apache.hadoop.ozone.shell.TestOzoneShellHA
-------------------------------------------------------------------------------
Tests run: 1, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 28.099 s - in org.apache.hadoop.ozone.shell.TestOzoneShellHA
**Manual test** through docker cluster deployment:
**before patch** ->
1. configure core-site.xml to use TrashPolicyDefault `<property>
<name>fs.trash.classname</name>
<value>org.apache.hadoop.ozone.om.TrashPolicyDefault</value>
</property>` OR leave unchanged (default behavior)
enable Trash
`<property>
<name>fs.trash.interval</name>
<value>60</value>
</property>`
2. run shell commands to create volume, bucket and key (eg. /vol1/bucket1/key1)
3. run shell command to delete key (eg. `bin/ozone fs -rm -R o3fs://bucket1.vol1/key1`)
4. run shell command to delete trash (eg. `bin/ozone fs -rm -R o3fs://bucket1.vol1/.Trash`)
Trash directory remains.
**patch** ->
1. configure core-site.xml to use TrashPolicyOzone `<property>
<name>fs.trash.classname</name>
<value>org.apache.hadoop.ozone.om.TrashPolicyOzone</value>
</property>`
enable Trash
`<property>
<name>fs.trash.interval</name>
<value>60</value>
</property>`
2. run shell commands to create volume, bucket and key (eg. /vol1/bucket1/key1)
3. run shell command to delete key (eg. `bin/ozone fs -rm -R o3fs://bucket1.vol1/key1`)
4. run shell command to delete trash (eg. `bin/ozone fs -rm -R o3fs://bucket1.vol1/.Trash`)
Trash directory **_is deleted_**.
--
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
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
[GitHub] [ozone] elek commented on pull request #2110: HDDS-4043. allow deletion from Trash directory without -skipTrash option
Posted by GitBox <gi...@apache.org>.
elek commented on pull request #2110:
URL: https://github.com/apache/ozone/pull/2110#issuecomment-849584311
Thanks for the review @sadanand48 and the patch @neils-dev. Merging it now.
--
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
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
[GitHub] [ozone] neils-dev commented on pull request #2110: HDDS-4043. allow deletion from Trash directory without -skipTrash option
Posted by GitBox <gi...@apache.org>.
neils-dev commented on pull request #2110:
URL: https://github.com/apache/ozone/pull/2110#issuecomment-816394895
Thanks @sadanand48 for reviewing the PR and the great comments! Latest commit with changes for comments.
--
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
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
[GitHub] [ozone] neils-dev commented on pull request #2110: HDDS-4043. allow deletion from Trash directory without -skipTrash option
Posted by GitBox <gi...@apache.org>.
neils-dev commented on pull request #2110:
URL: https://github.com/apache/ozone/pull/2110#issuecomment-847514044
Recent commit contains fix for problem running trash service integration tests consecutively (repeat until failure). The OmTransport RPC connection between the shell and om was not closed at end of test; improper cleanup prior to start of next test. Fixed with calling the close() method of the filesystem class implementation.
--
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
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
[GitHub] [ozone] elek commented on pull request #2110: HDDS-4043. allow deletion from Trash directory without -skipTrash option
Posted by GitBox <gi...@apache.org>.
elek commented on pull request #2110:
URL: https://github.com/apache/ozone/pull/2110#issuecomment-831828608
Thanks the patch @neils-dev. It looks good to me, but I tried to run the Unit test locally before the commit.
When I try to debug it the unit test is frequently failing:
```
java.lang.AssertionError:
Expected :0
Actual :1
<Click to see difference>
at org.junit.Assert.fail(Assert.java:89)
at org.junit.Assert.failNotEquals(Assert.java:835)
at org.junit.Assert.assertEquals(Assert.java:647)
at org.junit.Assert.assertEquals(Assert.java:633)
at org.apache.hadoop.ozone.shell.TestOzoneShellHA.testDeleteTrashNoSkipTrash(TestOzoneShellHA.java:635)
at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62)
at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
at java.lang.reflect.Method.invoke(Method.java:498)
at org.junit.runners.model.FrameworkMethod$1.runReflectiveCall(FrameworkMethod.java:59)
at org.junit.internal.runners.model.ReflectiveCallable.run(ReflectiveCallable.java:12)
at org.junit.runners.model.FrameworkMethod.invokeExplosively(FrameworkMethod.java:56)
at org.junit.internal.runners.statements.InvokeMethod.evaluate(InvokeMethod.java:17)
at org.junit.internal.runners.statements.RunBefores.evaluate(RunBefores.java:26)
at org.junit.internal.runners.statements.RunAfters.evaluate(RunAfters.java:27)
at org.junit.internal.runners.statements.FailOnTimeout$CallableStatement.call(FailOnTimeout.java:288)
at org.junit.internal.runners.statements.FailOnTimeout$CallableStatement.call(FailOnTimeout.java:282)
at java.util.concurrent.FutureTask.run$$$capture(FutureTask.java:266)
at java.util.concurrent.FutureTask.run(FutureTask.java)
at java.lang.Thread.run(Thread.java:748)
```
So I tried to run the unit with IntelliJ "repeat until fail" functionality, but it is always failed at the second time.
Do you have the same behavior? Can you run it multiple times without any error?
I would like to be sure that it's not an intermittent test.
--
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
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
[GitHub] [ozone] sadanand48 commented on a change in pull request #2110: HDDS-4043. allow deletion from Trash directory without -skipTrash option
Posted by GitBox <gi...@apache.org>.
sadanand48 commented on a change in pull request #2110:
URL: https://github.com/apache/ozone/pull/2110#discussion_r613041797
##########
File path: hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/shell/TestOzoneShellHA.java
##########
@@ -558,6 +560,86 @@ public void testDeleteToTrashOrSkipTrash() throws Exception {
}
}
+ @Test
+ public void testDeleteTrashNoSkipTrash() throws Exception {
+
+ // Test delete from Trash directory removes item from filesystem
+
+ // setup configuration to use TrashPolicyOzone
+ // (default is TrashPolicyDefault)
Review comment:
Thanks @neils-dev for making the change . Sorry , I wasn't clear . I meant to use the same URI as that of `testDeleteToTrashOrSkipTrash` i.e to include serviceId as part of the URI. Since this is HA setup ,its required to pass the serviceID. Please use the same URI as that of `testDeleteToTrashOrSkipTrash` i.e
```
final String hostPrefix = OZONE_OFS_URI_SCHEME + "://" + omServiceId;
OzoneConfiguration clientConf = getClientConfForOFS(hostPrefix,conf);
```
instead of
`OzoneConfiguration clientConf =
getClientConfForOzoneTrashPolicy("ofs://localhost", conf);`
--
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
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
[GitHub] [ozone] sadanand48 commented on a change in pull request #2110: HDDS-4043. allow deletion from Trash directory without -skipTrash option
Posted by GitBox <gi...@apache.org>.
sadanand48 commented on a change in pull request #2110:
URL: https://github.com/apache/ozone/pull/2110#discussion_r611000632
##########
File path: hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/TrashPolicyOzone.java
##########
@@ -118,6 +120,25 @@ public Runnable getEmptier() throws IOException {
emptierInterval);
}
+ @Override
+ public boolean moveToTrash(Path path) throws IOException {
+ this.fs.getFileStatus(path);
+ Path trashRoot = this.fs.getTrashRoot(path);
+ Path trashCurrent = new Path(trashRoot, CURRENT);
+
+ String key = path.toUri().getPath();
+ String trashRootKey = trashRoot.toUri().getPath();
+
+ if (!OzoneFSUtils.isValidName(key)) {
+ throw new InvalidPathException("Invalid path Name " + key);
+ }
+
+ if (trashRootKey.startsWith(key) || key.startsWith(trashRootKey)) {
Review comment:
Thanks for clarifying this @neils-dev . I forgot to consider the username in trashroot. It makes sense now . Thanks
--
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
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
[GitHub] [ozone] neils-dev edited a comment on pull request #2110: HDDS-4043. allow deletion from Trash directory without -skipTrash option
Posted by GitBox <gi...@apache.org>.
neils-dev edited a comment on pull request #2110:
URL: https://github.com/apache/ozone/pull/2110#issuecomment-834825807
Thanks for looking into potential problems with integration test **@elek**. I can confirm the error you see on your local test environment, however the messages I see associated with running the test 'repeat until fail' are timeout errors - `2021-05-07 15:51:56,695 [Time-limited test] ERROR ha.OMFailoverProxyProvider (OMFailoverProxyProvider.java:getRetryAction(294)) - Failed to connect to OMs: [nodeId=omNode-3,nodeAddress=127.0.0.1:13660, nodeId=omNode-1,nodeAddress=127.0.0.1:13648, nodeId=omNode-2,nodeAddress=127.0.0.1:13654]. Attempted 15 failovers. Got following exceptions during retries: `; followed by the assertion error you posted.
In addition I tried the same test setup with "repeat until failure" option with Intellij on the `org.apache.hadoop.ozone.shell.TestOzoneShellHA.testDeleteToTrashOrSkipTrash` with same error.
It does not appear to be intermittent, rather a problem with running the tests continuously. It may be from parts of the test initialized globally for `org.apache.hadoop.ozone.shell.TestOzoneShellHA` (@Before, @After, @BeforeClass ...), hence are not reinitialized on each invocation of a specific test such as `testDeleteToTrashOrSkipTrash`. I will look further into this and update.
--
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
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
[GitHub] [ozone] sadanand48 commented on a change in pull request #2110: HDDS-4043. allow deletion from Trash directory without -skipTrash option
Posted by GitBox <gi...@apache.org>.
sadanand48 commented on a change in pull request #2110:
URL: https://github.com/apache/ozone/pull/2110#discussion_r610347763
##########
File path: hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/TrashPolicyOzone.java
##########
@@ -118,6 +120,25 @@ public Runnable getEmptier() throws IOException {
emptierInterval);
}
+ @Override
+ public boolean moveToTrash(Path path) throws IOException {
+ this.fs.getFileStatus(path);
+ Path trashRoot = this.fs.getTrashRoot(path);
+ Path trashCurrent = new Path(trashRoot, CURRENT);
+
+ String key = path.toUri().getPath();
+ String trashRootKey = trashRoot.toUri().getPath();
+
+ if (!OzoneFSUtils.isValidName(key)) {
+ throw new InvalidPathException("Invalid path Name " + key);
+ }
+
+ if (trashRootKey.startsWith(key) || key.startsWith(trashRootKey)) {
Review comment:
Thanks for the explaining it in the comment . Although , when length of key < length of trashroot , the key would not be inside `.Trash` directory.
Suppose key = vol/buck/.Trash/user/randomKey ,the trashroot here would be vol/buck/.Trash
So length of key is always greater than or equal to the length of trashroot and first condition would not be required.
--
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
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
[GitHub] [ozone] neils-dev commented on pull request #2110: HDDS-4043. allow deletion from Trash directory without -skipTrash option
Posted by GitBox <gi...@apache.org>.
neils-dev commented on pull request #2110:
URL: https://github.com/apache/ozone/pull/2110#issuecomment-846649960
The problem indeed involves running the TestOzoneShellHA trash tests repetitively continuously. The test cluster is initialized at the start of the suite of tests with the Om rpc server and Om rpc client in sync, however when the test is repeated as in this case, the rpc client becomes out of sync and uses the wrong port to access the Om rpc server - occurs with shell rpc submitRequest calls. Problem is identified. Will investigate further.
--
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
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
[GitHub] [ozone] sadanand48 commented on pull request #2110: HDDS-4043. allow deletion from Trash directory without -skipTrash option
Posted by GitBox <gi...@apache.org>.
sadanand48 commented on pull request #2110:
URL: https://github.com/apache/ozone/pull/2110#issuecomment-847558318
Thanks @neils-dev for identifying the issue. @elek Can you please take a look and merge the change?
--
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
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
[GitHub] [ozone] sadanand48 commented on a change in pull request #2110: HDDS-4043. allow deletion from Trash directory without -skipTrash option
Posted by GitBox <gi...@apache.org>.
sadanand48 commented on a change in pull request #2110:
URL: https://github.com/apache/ozone/pull/2110#discussion_r611002854
##########
File path: hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/shell/TestOzoneShellHA.java
##########
@@ -558,6 +560,86 @@ public void testDeleteToTrashOrSkipTrash() throws Exception {
}
}
+ @Test
+ public void testDeleteTrashNoSkipTrash() throws Exception {
+
+ // Test delete from Trash directory removes item from filesystem
+
+ // setup configuration to use TrashPolicyOzone
+ // (default is TrashPolicyDefault)
Review comment:
@neils-dev Please use `getClientConfForOFS(hostPrefix,conf)` for initialising `clientConf` here. The integration test is failing because of this
```
// Test delete from Trash directory removes item from filesystem
// setup configuration to use TrashPolicyOzone
// (default is TrashPolicyDefault)
final String hostPrefix = OZONE_OFS_URI_SCHEME + "://" + omServiceId;
OzoneConfiguration clientConf = getClientConfForOFS(hostPrefix,conf);
clientConf.setInt(FS_TRASH_INTERVAL_KEY, 60);
clientConf.setClass("fs.trash.classname", TrashPolicyOzone.class,
TrashPolicy.class);
OzoneFsShell shell = new OzoneFsShell(clientConf);
```
--
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
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
[GitHub] [ozone] neils-dev commented on a change in pull request #2110: HDDS-4043. allow deletion from Trash directory without -skipTrash option
Posted by GitBox <gi...@apache.org>.
neils-dev commented on a change in pull request #2110:
URL: https://github.com/apache/ozone/pull/2110#discussion_r610975746
##########
File path: hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/TrashPolicyOzone.java
##########
@@ -118,6 +120,25 @@ public Runnable getEmptier() throws IOException {
emptierInterval);
}
+ @Override
+ public boolean moveToTrash(Path path) throws IOException {
+ this.fs.getFileStatus(path);
+ Path trashRoot = this.fs.getTrashRoot(path);
+ Path trashCurrent = new Path(trashRoot, CURRENT);
+
+ String key = path.toUri().getPath();
+ String trashRootKey = trashRoot.toUri().getPath();
+
+ if (!OzoneFSUtils.isValidName(key)) {
+ throw new InvalidPathException("Invalid path Name " + key);
+ }
+
+ if (trashRootKey.startsWith(key) || key.startsWith(trashRootKey)) {
Review comment:
Thanks @sadanand48, yes for what you described that is true. The trashroot is derived from the bucket under the key,` trashroot`=`/vol/bucket/.Trash/username`. One case where length `key` < length `trashroot` can be when `key` = `/vol/bucket/.Trash`; here `trashroot` is `/vol/bucket/.Trash/username`.
--
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
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
[GitHub] [ozone] neils-dev commented on pull request #2110: HDDS-4043. allow deletion from Trash directory without -skipTrash option
Posted by GitBox <gi...@apache.org>.
neils-dev commented on pull request #2110:
URL: https://github.com/apache/ozone/pull/2110#issuecomment-834825807
Thanks for looking into potential problems with integration test **@elek**. I can confirm the error you see on your local test environment, however the messages I see associated with running the test 'repeat until fail' are timeout errors - `2021-05-07 15:51:56,695 [Time-limited test] ERROR ha.OMFailoverProxyProvider (OMFailoverProxyProvider.java:getRetryAction(294)) - Failed to connect to OMs: [nodeId=omNode-3,nodeAddress=127.0.0.1:13660, nodeId=omNode-1,nodeAddress=127.0.0.1:13648, nodeId=omNode-2,nodeAddress=127.0.0.1:13654]. Attempted 15 failovers. Got following exceptions during retries: `
In addition I tried the same test setup with "repeat until failure" option with Intellij on the `org.apache.hadoop.ozone.shell.TestOzoneShellHA.testDeleteToTrashOrSkipTrash` with same error.
It does not appear to be intermittent, rather a problem with running the tests continuously. It may be from parts of the test initialized globally for `org.apache.hadoop.ozone.shell.TestOzoneShellHA` (@Before, @After, @BeforeClass ...), hence are not reinitialized on each invocation of a specific test such as `testDeleteToTrashOrSkipTrash`. I will look further into this and update.
--
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
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
[GitHub] [ozone] sadanand48 commented on a change in pull request #2110: HDDS-4043. allow deletion from Trash directory without -skipTrash option
Posted by GitBox <gi...@apache.org>.
sadanand48 commented on a change in pull request #2110:
URL: https://github.com/apache/ozone/pull/2110#discussion_r613041797
##########
File path: hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/shell/TestOzoneShellHA.java
##########
@@ -558,6 +560,86 @@ public void testDeleteToTrashOrSkipTrash() throws Exception {
}
}
+ @Test
+ public void testDeleteTrashNoSkipTrash() throws Exception {
+
+ // Test delete from Trash directory removes item from filesystem
+
+ // setup configuration to use TrashPolicyOzone
+ // (default is TrashPolicyDefault)
Review comment:
Thanks @neils-dev for making the change . Sorry , I wasn't clear . I meant to use the same URI as that of `testDeleteToTrashOrSkipTrash` i.e to include serviceId as part of the URI. Since this is HA setup ,its required to pass the serviceID. Please use the same URI as that of `testDeleteToTrashOrSkipTrash` i.e
```
final String hostPrefix = OZONE_OFS_URI_SCHEME + "://" + omServiceId;
OzoneConfiguration clientConf = getClientConfForOFS(hostPrefix,conf);
```
instead of
`OzoneConfiguration clientConf =
getClientConfForOzoneTrashPolicy("ofs://localhost", conf);`
I tried running the individual test in IDE and it's failing. The test only passes when the entire test file is run which is why the integration test passed.
--
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
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
[GitHub] [ozone] elek merged pull request #2110: HDDS-4043. allow deletion from Trash directory without -skipTrash option
Posted by GitBox <gi...@apache.org>.
elek merged pull request #2110:
URL: https://github.com/apache/ozone/pull/2110
--
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
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
[GitHub] [ozone] neils-dev commented on a change in pull request #2110: HDDS-4043. allow deletion from Trash directory without -skipTrash option
Posted by GitBox <gi...@apache.org>.
neils-dev commented on a change in pull request #2110:
URL: https://github.com/apache/ozone/pull/2110#discussion_r610339001
##########
File path: hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/TrashPolicyOzone.java
##########
@@ -118,6 +120,25 @@ public Runnable getEmptier() throws IOException {
emptierInterval);
}
+ @Override
+ public boolean moveToTrash(Path path) throws IOException {
+ this.fs.getFileStatus(path);
+ Path trashRoot = this.fs.getTrashRoot(path);
+ Path trashCurrent = new Path(trashRoot, CURRENT);
Review comment:
Removed unused variable.
--
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
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
[GitHub] [ozone] neils-dev commented on a change in pull request #2110: HDDS-4043. allow deletion from Trash directory without -skipTrash option
Posted by GitBox <gi...@apache.org>.
neils-dev commented on a change in pull request #2110:
URL: https://github.com/apache/ozone/pull/2110#discussion_r612932630
##########
File path: hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/shell/TestOzoneShellHA.java
##########
@@ -558,6 +560,86 @@ public void testDeleteToTrashOrSkipTrash() throws Exception {
}
}
+ @Test
+ public void testDeleteTrashNoSkipTrash() throws Exception {
+
+ // Test delete from Trash directory removes item from filesystem
+
+ // setup configuration to use TrashPolicyOzone
+ // (default is TrashPolicyDefault)
Review comment:
Thanks for the comment @sadanand48. Commit to set configuration in function setting TrashPolicyOzone after getClientConfForOFS as suggested.
--
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
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
[GitHub] [ozone] neils-dev commented on a change in pull request #2110: HDDS-4043. allow deletion from Trash directory without -skipTrash option
Posted by GitBox <gi...@apache.org>.
neils-dev commented on a change in pull request #2110:
URL: https://github.com/apache/ozone/pull/2110#discussion_r610339674
##########
File path: hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/TrashPolicyOzone.java
##########
@@ -118,6 +120,25 @@ public Runnable getEmptier() throws IOException {
emptierInterval);
}
+ @Override
+ public boolean moveToTrash(Path path) throws IOException {
+ this.fs.getFileStatus(path);
+ Path trashRoot = this.fs.getTrashRoot(path);
+ Path trashCurrent = new Path(trashRoot, CURRENT);
+
+ String key = path.toUri().getPath();
+ String trashRootKey = trashRoot.toUri().getPath();
+
+ if (!OzoneFSUtils.isValidName(key)) {
+ throw new InvalidPathException("Invalid path Name " + key);
+ }
+
+ if (trashRootKey.startsWith(key) || key.startsWith(trashRootKey)) {
Review comment:
Thanks for asking to clarify. Added comments in code to explain. In process also improved conditional to satisfy an edge condition where key = .Tra (and the trash prefix is .Trash that would cause an unintentional action, by deleting and not moving to trash).
```
// first condition tests when length key is <= length trash
// and second when length key > length trash
if ((key.contains(this.fs.TRASH_PREFIX)) && (trashRootKey.startsWith(key))
|| key.startsWith(trashRootKey)) {
```
--
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
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
[GitHub] [ozone] sadanand48 commented on a change in pull request #2110: HDDS-4043. allow deletion from Trash directory without -skipTrash option
Posted by GitBox <gi...@apache.org>.
sadanand48 commented on a change in pull request #2110:
URL: https://github.com/apache/ozone/pull/2110#discussion_r611002854
##########
File path: hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/shell/TestOzoneShellHA.java
##########
@@ -558,6 +560,86 @@ public void testDeleteToTrashOrSkipTrash() throws Exception {
}
}
+ @Test
+ public void testDeleteTrashNoSkipTrash() throws Exception {
+
+ // Test delete from Trash directory removes item from filesystem
+
+ // setup configuration to use TrashPolicyOzone
+ // (default is TrashPolicyDefault)
Review comment:
Please use `getClientConfForOFS(hostPrefix,conf)` for initialising `clientConf` here. The integration test is failing because of this
```
// Test delete from Trash directory removes item from filesystem
// setup configuration to use TrashPolicyOzone
// (default is TrashPolicyDefault)
final String hostPrefix = OZONE_OFS_URI_SCHEME + "://" + omServiceId;
OzoneConfiguration clientConf = getClientConfForOFS(hostPrefix,conf);
clientConf.setInt(FS_TRASH_INTERVAL_KEY, 60);
clientConf.setClass("fs.trash.classname", TrashPolicyOzone.class,
TrashPolicy.class);
OzoneFsShell shell = new OzoneFsShell(clientConf);
```
--
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
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
[GitHub] [ozone] sadanand48 commented on a change in pull request #2110: HDDS-4043. allow deletion from Trash directory without -skipTrash option
Posted by GitBox <gi...@apache.org>.
sadanand48 commented on a change in pull request #2110:
URL: https://github.com/apache/ozone/pull/2110#discussion_r610347763
##########
File path: hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/TrashPolicyOzone.java
##########
@@ -118,6 +120,25 @@ public Runnable getEmptier() throws IOException {
emptierInterval);
}
+ @Override
+ public boolean moveToTrash(Path path) throws IOException {
+ this.fs.getFileStatus(path);
+ Path trashRoot = this.fs.getTrashRoot(path);
+ Path trashCurrent = new Path(trashRoot, CURRENT);
+
+ String key = path.toUri().getPath();
+ String trashRootKey = trashRoot.toUri().getPath();
+
+ if (!OzoneFSUtils.isValidName(key)) {
+ throw new InvalidPathException("Invalid path Name " + key);
+ }
+
+ if (trashRootKey.startsWith(key) || key.startsWith(trashRootKey)) {
Review comment:
Thanks for the explaining it in the comment . Although , when length of key < length of trashroot , the key would not be inside `.Trash` directory.
Suppose key = vol/buck/.Trash/user/randomKey ,the trashroot here would be vol/buck/.Trash
So length of key is always greater than length of trashroot and first condition would not be required.
--
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
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
[GitHub] [ozone] sadanand48 commented on a change in pull request #2110: HDDS-4043. allow deletion from Trash directory without -skipTrash option
Posted by GitBox <gi...@apache.org>.
sadanand48 commented on a change in pull request #2110:
URL: https://github.com/apache/ozone/pull/2110#discussion_r607757357
##########
File path: hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/TrashPolicyOzone.java
##########
@@ -118,6 +120,25 @@ public Runnable getEmptier() throws IOException {
emptierInterval);
}
+ @Override
+ public boolean moveToTrash(Path path) throws IOException {
+ this.fs.getFileStatus(path);
+ Path trashRoot = this.fs.getTrashRoot(path);
+ Path trashCurrent = new Path(trashRoot, CURRENT);
Review comment:
Please remove `trashCurrent `variable as it is unused
##########
File path: hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/TrashPolicyOzone.java
##########
@@ -118,6 +120,25 @@ public Runnable getEmptier() throws IOException {
emptierInterval);
}
+ @Override
+ public boolean moveToTrash(Path path) throws IOException {
+ this.fs.getFileStatus(path);
+ Path trashRoot = this.fs.getTrashRoot(path);
+ Path trashCurrent = new Path(trashRoot, CURRENT);
+
+ String key = path.toUri().getPath();
+ String trashRootKey = trashRoot.toUri().getPath();
+
Review comment:
Can we please add a debug level log that prints values for` key` and `trashRootKey.`
##########
File path: hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/TrashPolicyOzone.java
##########
@@ -118,6 +120,25 @@ public Runnable getEmptier() throws IOException {
emptierInterval);
}
+ @Override
+ public boolean moveToTrash(Path path) throws IOException {
+ this.fs.getFileStatus(path);
+ Path trashRoot = this.fs.getTrashRoot(path);
+ Path trashCurrent = new Path(trashRoot, CURRENT);
+
+ String key = path.toUri().getPath();
+ String trashRootKey = trashRoot.toUri().getPath();
+
+ if (!OzoneFSUtils.isValidName(key)) {
+ throw new InvalidPathException("Invalid path Name " + key);
+ }
+
+ if (trashRootKey.startsWith(key) || key.startsWith(trashRootKey)) {
Review comment:
I think only `key.startsWith(trashRootKey)` check would suffice. Do we need the OR condition?
--
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
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
[GitHub] [ozone] neils-dev commented on a change in pull request #2110: HDDS-4043. allow deletion from Trash directory without -skipTrash option
Posted by GitBox <gi...@apache.org>.
neils-dev commented on a change in pull request #2110:
URL: https://github.com/apache/ozone/pull/2110#discussion_r613444873
##########
File path: hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/shell/TestOzoneShellHA.java
##########
@@ -558,6 +560,86 @@ public void testDeleteToTrashOrSkipTrash() throws Exception {
}
}
+ @Test
+ public void testDeleteTrashNoSkipTrash() throws Exception {
+
+ // Test delete from Trash directory removes item from filesystem
+
+ // setup configuration to use TrashPolicyOzone
+ // (default is TrashPolicyDefault)
Review comment:
Thanks for your comment **@sadanand48** . Commit a change for the service id as suggested. Thanks for following up. BTW, the host _set to_ `localhost`, as was previously used for the clientconf _should not_ be a problem with the integration test. It was tested both individually` hadoop-ozone/integration-test$ mvn -Dtest=TestOzoneShellHA#testDeleteTrashNoSkipTrash test` and when the entire suite is run, `hadoop-ozone/integration-test$ mvn -Dtest=TestOzoneShellHA test`. See attached image.
` hadoop-ozone/integration-test$ mvn -Dtest=TestOzoneShellHA#testDeleteTrashNoSkipTrash test`
--
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
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org