Skip to content

Commit

Permalink
core: update NameSpaceProvider#getParentOf to include name of target …
Browse files Browse the repository at this point in the history
…in parent

Motivation:

The getParentOf method in NameSpaceProvider suffers from two limitations:
  1. lack of support for hard links,
  2. fails to include the name of the target file or directory.

Modification:

Update the method to return both the parent directory ID and the name of
the target.  The return is also updated to support multiple responses.

Chimera implementation of NameSpaceProvider is updated, following a
similar development to support this query.

The RemoteNameSpaceProvider is updated by updating PnfsManager, the cell
message that queries PnfsManager, and PnfsManager itself.

Result:

No admin- or user observable changes, but the NameSpaceProvider
interface is more powerful.

Target: master
Require-notes: no
Require-book: no
Patch: https://rb.dcache.org/r/10929/
Acked-by: Tigran Mkrtchyan
  • Loading branch information
paulmillar committed May 4, 2018
1 parent e47c9ab commit 510b522
Show file tree
Hide file tree
Showing 8 changed files with 117 additions and 27 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,6 @@
import java.net.URI;
import java.net.URISyntaxException;
import java.util.ArrayList;
import java.util.Collections;
import java.util.EnumSet;
import java.util.List;
import java.util.Objects;
Expand All @@ -44,10 +43,8 @@
import java.util.concurrent.ScheduledFuture;
import java.util.concurrent.TimeUnit;

import diskCacheV111.util.AccessLatency;
import diskCacheV111.util.CacheException;
import diskCacheV111.util.PnfsId;
import diskCacheV111.util.RetentionPolicy;
import diskCacheV111.vehicles.StorageInfo;

import org.dcache.auth.Subjects;
Expand Down Expand Up @@ -85,7 +82,7 @@ private enum Operation
CREATE_ENTRY("createentry", "Create a new file entry in the pool"),
DELETE_ENTRY("deleteentry", "Removes file entry from the pool"),
PNFS_ID_TO_PATH("pnfsidtopath", "Reads path of the file"),
GET_PARENT("getparent", "Reads the parent pnfsid of the file"),
FIND("find", "Locates the file in the namespace"),
ADD_CHECKSUM("addchecksum", "Adds the given checksum to the file"),
GET_CHECKSUMS("getchecksums", "Reads all the checksums of the file"),
SET_FILE_ATTR("setfileattr", "Updates the file attributes"),
Expand Down Expand Up @@ -285,8 +282,8 @@ private void processOperation(Operation aOp, String path)
case PNFS_ID_TO_PATH:
provider.pnfsidToPath(Subjects.ROOT, getPnfsid(path));
break;
case GET_PARENT:
provider.getParentOf(Subjects.ROOT, getPnfsid(path));
case FIND:
provider.find(Subjects.ROOT, getPnfsid(path));
break;
case ADD_CHECKSUM:
provider.setFileAttributes(Subjects.ROOT, getPnfsid(path),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,8 @@
import java.util.concurrent.atomic.AtomicInteger;
import java.util.function.Consumer;
import java.util.regex.Pattern;
import java.util.stream.Collectors;
import java.util.stream.Stream;

import diskCacheV111.namespace.NameSpaceProvider;
import diskCacheV111.util.AccessLatency;
Expand Down Expand Up @@ -829,14 +831,27 @@ public void getInfo(PrintWriter pw)
}

@Override
public PnfsId getParentOf(Subject subject, PnfsId pnfsId) throws CacheException {
public Collection<Link> find(Subject subject, PnfsId pnfsId) throws CacheException {
try {
ExtendedInode inodeOfResource = new ExtendedInode(_fs, pnfsId, NO_STAT);
ExtendedInode inodeParent = inodeOfResource.getParent();
if (inodeParent == null) {
ExtendedInode target = new ExtendedInode(_fs, pnfsId, NO_STAT);

Collection<Link> locations = _fs.find(target).stream()
.flatMap(l -> {
try {
ExtendedInode parent = new ExtendedInode(_fs, l.getParent());
PnfsId parentId = parent.getPnfsId();
return Stream.of(new Link(parentId, l.getName()));
} catch (ChimeraFsException e) {
_log.error("Failed to find PnfsId of parent {}: {}",
l.getParent(), e.toString());
return Stream.of();
}
})
.collect(Collectors.toList());
if (locations.isEmpty()) {
throw new FileNotFoundCacheException("No such file or directory: " + pnfsId);
}
return inodeParent.getPnfsId();
return locations;
} catch (FileNotFoundHimeraFsException e) {
throw new FileNotFoundCacheException("No such file or directory: " + pnfsId);
} catch (IOException e) {
Expand Down
Original file line number Diff line number Diff line change
@@ -1,28 +1,54 @@
package diskCacheV111.vehicles;

import java.util.ArrayList;
import java.util.Collections;
import java.util.List;

import diskCacheV111.util.PnfsId;

public class PnfsGetParentMessage extends PnfsMessage
{
private static final long serialVersionUID = 7665073616113975562L;
private PnfsId _parent;
private List<PnfsId> parents;
private List<String> names;

public PnfsGetParentMessage(PnfsId pnfsId)
{
super(pnfsId);
setReplyRequired(true);
}

public void setParent(PnfsId parent)
public void addLocation(PnfsId parent, String name)
{
_parent = parent;
if (parents == null) {
parents = new ArrayList<>();
}
if (names == null) {
names = new ArrayList<>();
}
if (_parent == null) {
_parent = parent;
}
parents.add(parent);
names.add(name);
}

public PnfsId getParent()
{
return _parent;
}

public List<PnfsId> getParents()
{
return parents == null ? Collections.emptyList() : parents;
}

public List<String> getNames()
{
return names == null ? Collections.emptyList() : names;
}

@Override
public boolean invalidates(Message message)
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,33 @@
*/
public interface NameSpaceProvider
{
/**
* A Link represents the appearance of a file or directory within the
* namespace. The parent is always a directory. Listing that directory
* will yield the target with the given name.
*/
public class Link
{
private final PnfsId parent;
private final String name;

public Link(PnfsId parent, String name)
{
this.parent = parent;
this.name = name;
}

public PnfsId getParent()
{
return parent;
}

public String getName()
{
return name;
}
}

/**
* When the mode field is not specified in createEntry, the mode
* is inherited from the parent directory. When creating new
Expand Down Expand Up @@ -141,7 +168,17 @@ void rename(Subject subject, @Nullable PnfsId pnfsId, String sourcePath, String
String pnfsidToPath(Subject subject, PnfsId pnfsId) throws CacheException;
PnfsId pathToPnfsid(Subject subject, String path, boolean followLinks) throws CacheException;

PnfsId getParentOf(Subject subject, PnfsId pnfsId) throws CacheException;
/**
* Find the locations of the target file or directory within the namespace.
* If the target is a directory then there is (at most) one location. If
* the target is a file then more than one location is returned if the file
* has hard links.
* @param subject Subject of the user who invoked this method
* @param pnfsId the target to locate
* @return The locations where the target may be found.
* @throws CacheException
*/
Collection<Link> find(Subject subject, PnfsId pnfsId) throws CacheException;

void removeFileAttribute(Subject subject, PnfsId pnfsId, String attribute) throws CacheException;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@
import java.util.concurrent.LinkedBlockingQueue;
import java.util.concurrent.TimeUnit;

import diskCacheV111.namespace.NameSpaceProvider.Link;
import diskCacheV111.util.CacheException;
import diskCacheV111.util.FileNotFoundCacheException;
import diskCacheV111.util.FsPath;
Expand Down Expand Up @@ -1431,7 +1432,8 @@ private void getParent(PnfsGetParentMessage msg)
try {
PnfsId pnfsId = populatePnfsId(msg);
checkMask(msg);
msg.setParent(_nameSpaceProvider.getParentOf(msg.getSubject(), pnfsId));
_nameSpaceProvider.find(msg.getSubject(), pnfsId)
.forEach(l -> msg.addLocation(l.getParent(), l.getName()));
} catch (CacheException e) {
_log.warn(e.toString());
msg.setFailed(e.getRc(), e.getMessage());
Expand Down
16 changes: 13 additions & 3 deletions modules/dcache/src/main/java/diskCacheV111/util/PnfsHandler.java
Original file line number Diff line number Diff line change
Expand Up @@ -6,12 +6,15 @@

import javax.security.auth.Subject;

import java.util.ArrayList;
import java.util.Collection;
import java.util.Collections;
import java.util.EnumSet;
import java.util.List;
import java.util.Set;
import java.util.concurrent.TimeUnit;

import diskCacheV111.namespace.NameSpaceProvider.Link;
import diskCacheV111.vehicles.PnfsAddCacheLocationMessage;
import diskCacheV111.vehicles.PnfsClearCacheLocationMessage;
import diskCacheV111.vehicles.PnfsCreateEntryMessage;
Expand Down Expand Up @@ -362,10 +365,17 @@ public PnfsCreateEntryMessage createPnfsEntry(String path,
return request(new PnfsCreateEntryMessage(path, attributes));
}

public PnfsId getParentOf(PnfsId pnfsId)
throws CacheException
public Collection<Link> find(PnfsId pnfsId) throws CacheException
{
return request(new PnfsGetParentMessage(pnfsId)).getParent();
PnfsGetParentMessage response = request(new PnfsGetParentMessage(pnfsId));
List<PnfsId> parents = response.getParents();
List<String> names = response.getNames();
int count = Math.min(parents.size(), names.size());
List<Link> locations = new ArrayList<>(count);
for (int i = 0; i < count; i++) {
locations.add(new Link(parents.get(i), names.get(i)));
}
return locations;
}

public PnfsId deletePnfsEntry(String path) throws CacheException
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -141,10 +141,9 @@ public PnfsId pathToPnfsid(Subject subject, String path,
}

@Override
public PnfsId getParentOf(Subject subject, PnfsId id) throws CacheException
public Collection<Link> find(Subject subject, PnfsId id) throws CacheException
{
PnfsHandler pnfs = new PnfsHandler(_pnfs, subject, Restrictions.none());
return pnfs.getParentOf(id);
return new PnfsHandler(_pnfs, subject, Restrictions.none()).find(id);
}

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@
import java.util.Map;
import java.util.concurrent.Executor;

import diskCacheV111.namespace.NameSpaceProvider.Link;
import diskCacheV111.util.CacheException;
import diskCacheV111.util.FileExistsCacheException;
import diskCacheV111.util.FileNotFoundCacheException;
Expand Down Expand Up @@ -422,29 +423,32 @@ public void shouldFaileWhenGetFileAttributesForMissingEntry()


@Test
public void shouldSucceedForGetParentOfExistingEntry() throws Exception
public void shouldSucceedForFindExistingEntry() throws Exception
{
givenSuccessfulResponse((Modifier<PnfsGetParentMessage>)
(r) -> r.setParent(ANOTHER_PNFSID));
(r) -> r.addLocation(ANOTHER_PNFSID, "file"));

PnfsId parent = _namespace.getParentOf(ROOT, A_PNFSID);
Collection<Link> locations = _namespace.find(ROOT, A_PNFSID);

PnfsGetParentMessage sent =
getSingleSendAndWaitMessage(PnfsGetParentMessage.class);
assertThat(sent.getReplyRequired(), is(true));
assertThat(sent.getSubject(), is(ROOT));
assertThat(sent.getPnfsId(), is(A_PNFSID));

assertThat(parent, is(ANOTHER_PNFSID));
assertThat(locations.size(), is(1));
Link location = locations.iterator().next();
assertThat(location.getParent(), is(ANOTHER_PNFSID));
assertThat(location.getName(), is("file"));
}


@Test(expected=FileNotFoundCacheException.class)
public void shouldFailForGetParentOfMissingEntry() throws Exception
public void shouldFailForFindMissingEntry() throws Exception
{
givenFailureResponse(FILE_NOT_FOUND);

_namespace.getParentOf(ROOT, A_PNFSID);
_namespace.find(ROOT, A_PNFSID);
}


Expand Down

0 comments on commit 510b522

Please sign in to comment.