From 68096908b5f508ed4fdeb9229d183a90211e2f9c Mon Sep 17 00:00:00 2001 From: Paul Millar Date: Thu, 12 Aug 2021 10:37:56 +0200 Subject: [PATCH] pnfsmanager: fix regression introduced with commit 61a4e8dc4e 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 --- .../main/java/org/dcache/chimera/JdbcFs.java | 23 +++++++++++++++++-- 1 file changed, 21 insertions(+), 2 deletions(-) diff --git a/modules/chimera/src/main/java/org/dcache/chimera/JdbcFs.java b/modules/chimera/src/main/java/org/dcache/chimera/JdbcFs.java index 28cf5a9d41d..e3b2929990b 100644 --- a/modules/chimera/src/main/java/org/dcache/chimera/JdbcFs.java +++ b/modules/chimera/src/main/java/org/dcache/chimera/JdbcFs.java @@ -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; @@ -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; @@ -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; } }