[
https://issues.apache.org/jira/browse/HDFS-16024?focusedWorklogId=600873&page=com.atlassian.jira.plugin.system.issuetabpanels:worklog-tabpanel#worklog-600873
]
ASF GitHub Bot logged work on HDFS-16024:
-----------------------------------------
Author: ASF GitHub Bot
Created on: 22/May/21 17:35
Start Date: 22/May/21 17:35
Worklog Time Spent: 10m
Work Description: goiri commented on a change in pull request #3009:
URL: https://github.com/apache/hadoop/pull/3009#discussion_r637429648
##########
File path:
hadoop-hdfs-project/hadoop-hdfs-rbf/src/main/java/org/apache/hadoop/hdfs/server/federation/resolver/MountTableResolver.java
##########
@@ -337,6 +343,36 @@ public void refreshEntries(final Collection<MountTable>
entries) {
this.init = true;
}
+ /**
+ * Check if PATH is the trail associated with the Trash.
+ *
+ * @param path A path.
+ */
+ private static boolean isTrashPath(String path) throws IOException {
+ Pattern pattern = Pattern.compile(getTrashRoot() + TRASH_PATTERN + "/");
+ return pattern.matcher(path).find();
+ }
+
+ private static String getTrashRoot() throws IOException {
+ // Gets the Trash directory for the current user.
+ return FileSystem.USER_HOME_PREFIX + "/" +
+ RouterRpcServer.getRemoteUser().getUserName() + "/" +
+ FileSystem.TRASH_PREFIX;
+ }
+
+ /**
+ * Subtract a BaseTrash to get a new path.
+ *
+ * @param path Path to check/insert.
+ * @return New remote location.
+ * @throws IOException If it cannot find the location.
+ */
+ private static String subtractBaseTrashPath(String path)
Review comment:
Can we add a few unit tests for this method?
##########
File path:
hadoop-hdfs-project/hadoop-hdfs-rbf/src/main/java/org/apache/hadoop/hdfs/server/federation/resolver/MountTableResolver.java
##########
@@ -381,18 +417,37 @@ public void clear() {
public PathLocation getDestinationForPath(final String path)
throws IOException {
verifyMountTable();
+ PathLocation res;
+ String tmpPath = path;
+ if (isTrashPath(tmpPath)) {
+ tmpPath = subtractBaseTrashPath(tmpPath);
+ }
+ String finalPath = tmpPath;
readLock.lock();
try {
if (this.locationCache == null) {
- return lookupLocation(path);
+ res = lookupLocation(finalPath);
+ } else {
+ Callable<? extends PathLocation> meh = new Callable<PathLocation>() {
Review comment:
It was there already but let's fix it :)
##########
File path:
hadoop-hdfs-project/hadoop-hdfs-rbf/src/main/java/org/apache/hadoop/hdfs/server/federation/resolver/MountTableResolver.java
##########
@@ -337,6 +343,36 @@ public void refreshEntries(final Collection<MountTable>
entries) {
this.init = true;
}
+ /**
+ * Check if PATH is the trail associated with the Trash.
+ *
+ * @param path A path.
+ */
+ private static boolean isTrashPath(String path) throws IOException {
Review comment:
Add a few tests.
##########
File path:
hadoop-hdfs-project/hadoop-hdfs-rbf/src/main/java/org/apache/hadoop/hdfs/server/federation/resolver/MountTableResolver.java
##########
@@ -381,18 +417,37 @@ public void clear() {
public PathLocation getDestinationForPath(final String path)
throws IOException {
verifyMountTable();
+ PathLocation res;
+ String tmpPath = path;
+ if (isTrashPath(tmpPath)) {
+ tmpPath = subtractBaseTrashPath(tmpPath);
+ }
+ String finalPath = tmpPath;
readLock.lock();
try {
if (this.locationCache == null) {
- return lookupLocation(path);
+ res = lookupLocation(finalPath);
+ } else {
+ Callable<? extends PathLocation> meh = new Callable<PathLocation>() {
+ @Override
+ public PathLocation call() throws Exception {
+ return lookupLocation(finalPath);
+ }
+ };
+ res = this.locationCache.get(finalPath, meh);
}
- Callable<? extends PathLocation> meh = new Callable<PathLocation>() {
- @Override
- public PathLocation call() throws Exception {
- return lookupLocation(path);
+ if (isTrashPath(path)) {
+ List<RemoteLocation> remoteLocations = new ArrayList<>();
+ for (RemoteLocation remoteLocation : res.getDestinations()) {
+ remoteLocations.add(
+ new RemoteLocation(remoteLocation.getNsId(),
Review comment:
It might be better to create a new constructor where we pass a
RemoteLocation and a path?
##########
File path:
hadoop-hdfs-project/hadoop-hdfs-rbf/src/main/java/org/apache/hadoop/hdfs/server/federation/resolver/MountTableResolver.java
##########
@@ -381,18 +417,37 @@ public void clear() {
public PathLocation getDestinationForPath(final String path)
throws IOException {
verifyMountTable();
+ PathLocation res;
+ String tmpPath = path;
+ if (isTrashPath(tmpPath)) {
+ tmpPath = subtractBaseTrashPath(tmpPath);
+ }
+ String finalPath = tmpPath;
readLock.lock();
try {
if (this.locationCache == null) {
- return lookupLocation(path);
+ res = lookupLocation(finalPath);
+ } else {
+ Callable<? extends PathLocation> meh = new Callable<PathLocation>() {
Review comment:
We should have something better than meh, and this is an ideal candidate
for using a lambda.
##########
File path:
hadoop-hdfs-project/hadoop-hdfs-rbf/src/main/java/org/apache/hadoop/hdfs/server/federation/resolver/MountTableResolver.java
##########
@@ -381,18 +417,37 @@ public void clear() {
public PathLocation getDestinationForPath(final String path)
throws IOException {
verifyMountTable();
+ PathLocation res;
+ String tmpPath = path;
Review comment:
This whole pass to finalPath could be a method and the names could be a
little more self-explanatory.
--
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:
[email protected]
Issue Time Tracking
-------------------
Worklog Id: (was: 600873)
Time Spent: 5h 50m (was: 5h 40m)
> RBF: Rename data to the Trash should be based on src locations
> --------------------------------------------------------------
>
> Key: HDFS-16024
> URL: https://issues.apache.org/jira/browse/HDFS-16024
> Project: Hadoop HDFS
> Issue Type: Improvement
> Components: rbf
> Affects Versions: 3.4.0
> Reporter: zhu
> Assignee: zhu
> Priority: Major
> Labels: pull-request-available
> Time Spent: 5h 50m
> Remaining Estimate: 0h
>
> 1.When deleting data to the Trash without configuring a mount point for the
> Trash, the Router should recognize and move the data to the Trash
> 2.When the user’s trash can is configured with a mount point and is different
> from the NS of the deleted directory, the router should identify and move the
> data to the trash can of the current user of src
> The same is true for using ViewFs mount points, I think we should be
> consistent with it
--
This message was sent by Atlassian Jira
(v8.3.4#803005)
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
