Skip to content

Commit

Permalink
Merge pull request #532 from pdowler/vos2
Browse files Browse the repository at this point in the history
vault node deletion
  • Loading branch information
pdowler authored Sep 8, 2023
2 parents c6dc837 + b3f51aa commit dc55e9a
Show file tree
Hide file tree
Showing 4 changed files with 180 additions and 46 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -541,6 +541,35 @@ public void testPutGetUpdateDeleteLinkNode() throws InterruptedException,
Assert.assertNull(gone);
}

@Test
public void testGetWithLock() {
UUID rootID = new UUID(0L, 0L);
ContainerNode root = new ContainerNode(rootID, "root");

// put
ContainerNode orig = new ContainerNode("container-test");
orig.parent = root;
orig.ownerID = "the-owner";
nodeDAO.put(orig);

// get-by-id
Node a = nodeDAO.get(orig.getID());
Assert.assertNotNull(a);
log.info("found by id: " + a.getID() + " aka " + a);
Assert.assertEquals(orig.getID(), a.getID());
Assert.assertEquals(orig.getName(), a.getName());
Assert.assertEquals(root.getID(), a.parentID);

// get with lock
Node locked = nodeDAO.lock(a);
Assert.assertNotNull(locked);
log.info("locked: " + a.getID() + " aka " + a);

nodeDAO.delete(orig.getID());
Node gone = nodeDAO.get(orig.getID());
Assert.assertNull(gone);
}

@Test
public void testContainerNodeIterator() throws IOException {
UUID rootID = new UUID(0L, 0L);
Expand All @@ -558,6 +587,8 @@ public void testContainerNodeIterator() throws IOException {
Assert.assertEquals(orig.getName(), a.getName());

Assert.assertTrue(a instanceof ContainerNode);
ContainerNode cn = (ContainerNode) a;
Assert.assertTrue(nodeDAO.isEmpty(cn));

// these are set in put
Assert.assertEquals(orig.getMetaChecksum(), a.getMetaChecksum());
Expand Down Expand Up @@ -588,10 +619,13 @@ public void testContainerNodeIterator() throws IOException {
link.ownerID = orig.ownerID;
log.info("put child: " + cont + " of " + cont.parent);
nodeDAO.put(cont);
Assert.assertFalse(nodeDAO.isEmpty(cn));
log.info("put child: " + data + " of " + data.parent);
nodeDAO.put(data);
Assert.assertFalse(nodeDAO.isEmpty(cn));
log.info("put child: " + link + " of " + link.parent);
nodeDAO.put(link);
Assert.assertFalse(nodeDAO.isEmpty(cn));

Node c1;
Node c2;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -108,6 +108,7 @@
import org.opencadc.vospace.NodeProperty;
import org.springframework.dao.DataAccessException;
import org.springframework.jdbc.core.JdbcTemplate;
import org.springframework.jdbc.core.PreparedStatementCreator;
import org.springframework.jdbc.core.ResultSetExtractor;
import org.springframework.jdbc.core.RowMapper;

Expand Down Expand Up @@ -304,17 +305,15 @@ public EntityGet<? extends Entity> getEntityGet(Class c, boolean forUpdate) {
return new StorageSiteGet(forUpdate);
}

if (Node.class.equals(c) || Node.class.isInstance(c)) {
if (Node.class.equals(c)) {
return new NodeGet(forUpdate);
}
if (DeletedNodeEvent.class.equals(c)) {
//return new DeletedNodeGet();
}

if (forUpdate) {
throw new UnsupportedOperationException("entity-get + forUpdate: " + c.getSimpleName());
}

// raw events are never locked for update
if (DeletedArtifactEvent.class.equals(c)) {
return new DeletedArtifactEventGet();
}
Expand All @@ -327,13 +326,40 @@ public EntityGet<? extends Entity> getEntityGet(Class c, boolean forUpdate) {
if (ObsoleteStorageLocation.class.equals(c)) {
return new ObsoleteStorageLocationGet();
}

if (DeletedNodeEvent.class.equals(c)) {
//return new DeletedNodeGet();
}

if (HarvestState.class.equals(c)) {
return new HarvestStateGet();
}

throw new UnsupportedOperationException("entity-get: " + c.getName());
}

public NodeCount getNodeCount() {
return new NodeCount();
}

public class NodeCount {
private UUID id;

public void setID(UUID id) {
this.id = id;
}

public int execute(JdbcTemplate jdbc) {
StringBuilder sb = new StringBuilder();
sb.append("SELECT count(*) FROM ").append(getTable(Node.class));
sb.append(" WHERE parentID = '").append(id.toString()).append("'");
String sql = sb.toString();
log.debug("NodeCount: " + sql);
int ret = jdbc.queryForObject(sql, Integer.class);
return ret;
}
}

public EntityIteratorQuery getEntityIteratorQuery(Class c) {
if (Artifact.class.equals(c)) {
return new ArtifactIteratorQuery();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -98,6 +98,15 @@ public void put(Node val) {
super.put(val);
}

@Override
public Node lock(Node n) {
if (n == null) {
throw new IllegalArgumentException("entity cannot be null");
}
// override because Node has subclasses: force base class here
return super.lock(Node.class, n.getID());
}

public Node get(UUID id) {
return super.get(Node.class, id);
}
Expand All @@ -121,6 +130,26 @@ public Node get(ContainerNode parent, String name) {
throw new RuntimeException("BUG: handleInternalFail did not throw");
}

public boolean isEmpty(ContainerNode parent) {
checkInit();
log.debug("isEmpty: " + parent.getID());
long t = System.currentTimeMillis();

try {
JdbcTemplate jdbc = new JdbcTemplate(dataSource);
SQLGenerator.NodeCount count = (SQLGenerator.NodeCount) gen.getNodeCount();
count.setID(parent.getID());
int num = count.execute(jdbc);
return (num == 0);
} catch (BadSqlGrammarException ex) {
handleInternalFail(ex);
} finally {
long dt = System.currentTimeMillis() - t;
log.debug("isEmpty: " + parent.getID() + " " + dt + "ms");
}
throw new RuntimeException("BUG: handleInternalFail did not throw");
}

public void delete(UUID id) {
super.delete(Node.class, id);
}
Expand Down
129 changes: 87 additions & 42 deletions vault/src/main/java/org/opencadc/vault/NodePersistenceImpl.java
Original file line number Diff line number Diff line change
Expand Up @@ -72,6 +72,7 @@
import ca.nrc.cadc.auth.PrincipalExtractor;
import ca.nrc.cadc.auth.X509CertificateChain;
import ca.nrc.cadc.date.DateUtil;
import ca.nrc.cadc.db.TransactionManager;
import ca.nrc.cadc.io.ResourceIterator;
import ca.nrc.cadc.net.TransientException;
import ca.nrc.cadc.util.InvalidConfigException;
Expand All @@ -91,8 +92,10 @@
import javax.security.auth.x500.X500Principal;
import org.apache.log4j.Logger;
import org.opencadc.inventory.Artifact;
import org.opencadc.inventory.DeletedArtifactEvent;
import org.opencadc.inventory.Namespace;
import org.opencadc.inventory.db.ArtifactDAO;
import org.opencadc.inventory.db.DeletedArtifactEventDAO;
import org.opencadc.inventory.db.SQLGenerator;
import org.opencadc.vospace.ContainerNode;
import org.opencadc.vospace.DataNode;
Expand All @@ -119,8 +122,6 @@ public class NodePersistenceImpl implements NodePersistence {
private final Set<URI> artifactProps = new TreeSet<>();
private URI resourceID;

private IdentityManager identityManager = AuthenticationUtil.getIdentityManager();

public NodePersistenceImpl(URI resourceID) {
if (resourceID == null) {
throw new IllegalArgumentException("resource ID required");
Expand Down Expand Up @@ -152,6 +153,7 @@ public X509CertificateChain getCertificateChain() {
return null;
}
});
IdentityManager identityManager = AuthenticationUtil.getIdentityManager();

// root node
UUID rootID = new UUID(0L, 0L);
Expand Down Expand Up @@ -262,9 +264,12 @@ public Node get(ContainerNode parent, String name) throws TransientException {
return null;
}
ret.parent = parent;
IdentityManager identityManager = AuthenticationUtil.getIdentityManager();
ret.owner = identityManager.toSubject(ret.ownerID);
ret.ownerDisplay = identityManager.toDisplayString(ret.owner);

// in principle we could have queried vospace.Node join inventory.Artifact above
// and avoid this query.... simplicity for now
if (ret instanceof DataNode) {
DataNode dn = (DataNode) ret;
ArtifactDAO artifactDAO = getArtifactDAO();
Expand Down Expand Up @@ -312,9 +317,22 @@ private class IdentWrapper implements ResourceIterator<Node> {
private final ContainerNode parent;
private final ResourceIterator<Node> childIter;

private IdentityManager identityManager = AuthenticationUtil.getIdentityManager();
private Map<Object, Subject> identCache = new TreeMap<>();

IdentWrapper(ContainerNode parent, ResourceIterator<Node> childIter) {
this.parent = parent;
this.childIter = childIter;
// prime cache with caller
Subject caller = AuthenticationUtil.getCurrentSubject();
if (caller != null) {
Object ownerID = identityManager.toOwner(caller);
if (ownerID != null) {
// HACK: NodeDAO returns ownerID as String and relies on the IM
// to convert to a number (eg)
identCache.put(ownerID.toString(), caller);
}
}
}

@Override
Expand All @@ -326,14 +344,20 @@ public boolean hasNext() {
public Node next() {
Node ret = childIter.next();
ret.parent = parent;
ret.owner = identityManager.toSubject(ret.ownerID);
Subject s = identCache.get(ret.ownerID);
if (s == null) {
s = identityManager.toSubject(ret.ownerID);
identCache.put(ret.ownerID, s);
}
ret.owner = s;
ret.ownerDisplay = identityManager.toDisplayString(ret.owner);
return ret;
}

@Override
public void close() throws IOException {
childIter.close();
identCache.clear();
}

}
Expand Down Expand Up @@ -376,6 +400,7 @@ public Node put(Node node) throws NodeNotSupportedException, TransientException
if (node.owner == null) {
throw new RuntimeException("BUG: cannot persist node without owner: " + node);
}
IdentityManager identityManager = AuthenticationUtil.getIdentityManager();
node.ownerID = identityManager.toOwner(node.owner);
}

Expand Down Expand Up @@ -454,48 +479,68 @@ public void delete(Node node) throws TransientException {
throw new IllegalArgumentException("arg cannot be null: node");
}

NodeDAO dao = getDAO();
final NodeDAO dao = getDAO();
final ArtifactDAO artifactDAO = getArtifactDAO();
TransactionManager txn = dao.getTransactionManager();

// TODO: do the following in a transaction, acquire lock on target node

boolean moveToTrash = true; // default
URI storageID = null;
if (node instanceof ContainerNode) {
ContainerNode cn = (ContainerNode) node;
try (ResourceIterator<Node> iter = dao.iterator(cn, 1, null)) {
moveToTrash = iter.hasNext(); // empty
} catch (IOException ex) {
throw new TransientException("database IO failure", ex);
}
} else if (node instanceof LinkNode) {
moveToTrash = false;
} else if (node instanceof DataNode) {
DataNode dn = (DataNode) node;
NodeProperty len = dn.getProperty(VOS.PROPERTY_URI_CONTENTLENGTH);
if (len == null) {
// artifact does not exist
moveToTrash = false;
try {
log.debug("starting transaction");
txn.startTransaction();
log.debug("start txn: OK");

Node locked = dao.lock(node);
if (locked != null) {
node = locked; // safer than having two vars and accidentally using the wrong one
URI storageID = null;
if (node instanceof ContainerNode) {
ContainerNode cn = (ContainerNode) node;
boolean empty = dao.isEmpty(cn);
if (!empty) {
log.debug("commit txn...");
txn.commitTransaction();
log.debug("commit txn: OK");
throw new IllegalArgumentException("container node '" + node.getName() + "' is not empty");
}
} else if (node instanceof DataNode) {
DataNode dn = (DataNode) node;
NodeProperty len = dn.getProperty(VOS.PROPERTY_URI_CONTENTLENGTH);
if (len != null) {
// artifact exists
storageID = dn.storageID;
}
} // else: LinkNode can always be deleted

if (storageID != null) {
Artifact a = artifactDAO.get(storageID);
if (a != null) {
DeletedArtifactEventDAO daeDAO = new DeletedArtifactEventDAO(artifactDAO);
DeletedArtifactEvent dae = new DeletedArtifactEvent(a.getID());
daeDAO.put(dae);
artifactDAO.delete(a.getID());
}
}
// TODO: need DeletedNodeDAO to create DeletedNodeEvent
dao.delete(node.getID());
} else {
storageID = dn.storageID;
log.debug("failed to lock node " + node.getID() + " - assume deleted by another process");
}

log.debug("commit txn...");
txn.commitTransaction();
log.debug("commit txn: OK");
} catch (Exception ex) {
if (txn.isOpen()) {
log.error("failed to delete " + node.getID() + " aka " + node.getName(), ex);
txn.rollbackTransaction();
log.debug("rollback txn: OK");
}
throw ex;
} finally {
if (txn.isOpen()) {
log.error("BUG - open transaction in finally");
txn.rollbackTransaction();
log.error("rollback txn: OK");
}
}

// TODO: create DeletedNodeEvent?
// need DeletedNodeDAO
// what about DNE for all child nodes, which also got deleted? or would sync of a
// DNE involve calling NodePersistence.delete(DeletedNodeEvent) to replay this same
// deletion logic in a mirror?? TBD
// persisting the DNE means that recovery from trash has to re-assign IDs

// TODO: if DataNode (storageID != null): delete artifact and create DeletedArtifactEvent?

if (moveToTrash) {
node.parentID = trash.getID();
node.setName(node.getName() + "-" + UUID.randomUUID().toString());
dao.put(node);
} else {
dao.delete(node.getID());
}

}
}

0 comments on commit dc55e9a

Please sign in to comment.