Skip to content

Commit

Permalink
pnfsmanager: fix regression introduced with commit 61a4e8d
Browse files Browse the repository at this point in the history
Motivation:

Observer stack-trace while copying data into /tape in system-test:

  12 Aug 2021 10:09:49 (PnfsManager) [door:WebDAV-NA-sprocket@dCacheDomain:AAXJWEJG0qA WebDAV-NA-sprocket PnfsCreateEntry] Bug found when creating entry:
  java.lang.IllegalArgumentException: Unknown Policy: "CUSTODIAL
  "
          at diskCacheV111.util.RetentionPolicy.getRetentionPolicy(RetentionPolicy.java:148)
          at diskCacheV111.util.RetentionPolicy.valueOf(RetentionPolicy.java:164)
          at org.dcache.chimera.JdbcFs.getRetentionPolicyFromParentTag(JdbcFs.java:1577)
          at org.dcache.chimera.JdbcFs.lambda$createFile$9(JdbcFs.java:434)
          at org.dcache.chimera.JdbcFs.inTransaction(JdbcFs.java:229)
          at org.dcache.chimera.JdbcFs.createFile(JdbcFs.java:421)
          at org.dcache.chimera.JdbcFs.createFile(JdbcFs.java:357)
          at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
          at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62)
          at java.base/jdk.internal.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
          at java.base/java.lang.reflect.Method.invoke(Method.java:566)
          at org.dcache.commons.stats.MonitoringProxy.invoke(MonitoringProxy.java:54)
          at com.sun.proxy.$Proxy33.createFile(Unknown Source)
          at org.dcache.chimera.FsInode.create(FsInode.java:261)
          at org.dcache.chimera.namespace.ExtendedInode.create(ExtendedInode.java:127)
          at org.dcache.chimera.namespace.ChimeraNameSpaceProvider.createFile(ChimeraNameSpaceProvider.java:321)
          at diskCacheV111.namespace.ForwardingNameSpaceProvider.createFile(ForwardingNameSpaceProvider.java:56)
          at diskCacheV111.namespace.MonitoringNameSpaceProvider.createFile(MonitoringNameSpaceProvider.java:181)
          at diskCacheV111.namespace.PnfsManagerV3.createEntry(PnfsManagerV3.java:1818)
          at diskCacheV111.namespace.PnfsManagerV3.processMessageTransactionally_aroundBody4(PnfsManagerV3.java:2369)
          at diskCacheV111.namespace.PnfsManagerV3$AjcClosure5.run(PnfsManagerV3.java:1)
          at org.springframework.transaction.aspectj.AbstractTransactionAspect.ajc$around$org_springframework_transaction_aspectj_AbstractTransactionAspect$1$2a73e96cproceed(AbstractTransactionAspect.aj:66)
          at org.springframework.transaction.aspectj.AbstractTransactionAspect$AbstractTransactionAspect$1.proceedWithInvocation(AbstractTransactionAspect.aj:72)
          at org.springframework.transaction.interceptor.TransactionAspectSupport.invokeWithinTransaction(TransactionAspectSupport.java:366)
          at org.springframework.transaction.aspectj.AbstractTransactionAspect.ajc$around$org_springframework_transaction_aspectj_AbstractTransactionAspect$1$2a73e96c(AbstractTransactionAspect.aj:70)
          at diskCacheV111.namespace.PnfsManagerV3.processMessageTransactionally(PnfsManagerV3.java:2362)
          at diskCacheV111.namespace.PnfsManagerV3.processPnfsMessage(PnfsManagerV3.java:2333)
          at diskCacheV111.namespace.PnfsManagerV3$ProcessThread.run(PnfsManagerV3.java:2248)
          at java.base/java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1128)
          at java.base/java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:628)
          at java.base/java.lang.Thread.run(Thread.java:829)

Modification:

The method `JdbcFs#getRetentionPolicyFromParentTag` provides a subset of
the functionality in `ChimeraHsmStorageInfoExtractor#getRetentionPolicy`
however, it also makes a couple of mistakes; specifically,

  1. it uses the whole tag's data, instead of only the first line.

  2. it doesn't anticipate that the value may be invalid.

These two problems are resolved with this patch; however, the underlying
DRY violation remains and should be addressed.

Result:

Fixed an unreleased regression where writing to tape fails with quota
enabled.

Target: master
Requires-notes: no
Requires-book: no
Patch: https://rb.dcache.org/r/13129/
Acked-by: Lea Morschel
  • Loading branch information
paulmillar committed Aug 14, 2021
1 parent c53315f commit 6809690
Showing 1 changed file with 21 additions and 2 deletions.
23 changes: 21 additions & 2 deletions modules/chimera/src/main/java/org/dcache/chimera/JdbcFs.java
Original file line number Diff line number Diff line change
Expand Up @@ -21,9 +21,11 @@
import com.google.common.cache.CacheBuilder;
import com.google.common.cache.CacheLoader;
import com.google.common.cache.LoadingCache;
import com.google.common.io.ByteSource;
import com.google.common.util.concurrent.ThreadFactoryBuilder;

import diskCacheV111.util.RetentionPolicy;

import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
import org.springframework.dao.DataAccessException;
Expand Down Expand Up @@ -63,7 +65,10 @@
import org.dcache.util.Checksum;

import static com.google.common.base.Preconditions.checkArgument;

import java.nio.charset.StandardCharsets;

import static java.nio.charset.StandardCharsets.UTF_8;
import static org.dcache.acl.enums.AceFlags.*;
import static org.dcache.chimera.FileSystemProvider.StatCacheOption.NO_STAT;
import static org.dcache.chimera.FileSystemProvider.StatCacheOption.STAT;
Expand Down Expand Up @@ -1568,16 +1573,30 @@ public FsInode getParent()
}
}

// REVISIT: this method violates DRY by duplicating functionality from
// ChimeraHsmStorageInfoExtractor#getRetentionPolicy.
private RetentionPolicy getRetentionPolicyFromParentTag(FsInode parent)
{
String firstLine;

try {
Stat tagStat = statTag(parent, "RetentionPolicy");
byte[] data = new byte[(int) tagStat.getSize()];
getTag(parent, "RetentionPolicy", data, 0, data.length);
RetentionPolicy rp = RetentionPolicy.valueOf(new String(data));
return rp;
firstLine = ByteSource.wrap(data).asCharSource(UTF_8).readFirstLine();
} catch (ChimeraFsException e) {
return _defaultRetentionPolicy;
} catch (IOException e) {
// This should not happen.
throw new RuntimeException("Unexpected IOException found", e);
}

try {
return RetentionPolicy.valueOf(firstLine);
} catch (IllegalArgumentException e) {
LOGGER.error("Badly formatted RetentionPolicy tag in {}: {}", parent,
e.getMessage());
return _defaultRetentionPolicy;
}
}

Expand Down

0 comments on commit 6809690

Please sign in to comment.