HPCC-17083 Spurious 'different version of index already loaded' error

The jhtree cache was keeping some IFileIO objects loaded and thus preventing
Roxie from accessing updated ones. Release any matches and retry the cache
lookup after a failure.

Signed-off-by: Richard Chapman <rchapman@hpccsystems.com>

roxie/ccd/ccdfile.cpp

@@ -1150,25 +1150,35 @@ public:
                 if ((dfsSize != (offset_t) -1 && dfsSize != f->getSize()) ||
                     (!dfsDate.isNull() && !dfsDate.equals(*f->queryDateTime(), false)))
                 {
-                    StringBuffer modifiedDt;
-                    if (!dfsDate.isNull())
-                        dfsDate.getString(modifiedDt);
-                    StringBuffer fileDt;
-                    f->queryDateTime()->getString(fileDt);
-                    if (fileErrorList.find(lfn) == 0)
+                    if (fileType == ROXIE_KEY)
                     {
-                        switch (fileType)
+                        // jhtree cache can keep files active and thus prevent us from loading a new version
+                        clearKeyStoreCacheEntry(f);  // Will release iff that is the only link
+                        f.clear(); // Note - needs to be done before calling getValue() again, hence the need to make it separate from the f.set below
+                        f.set(files.getValue(localLocation));
+                    }
+                    if (f)  // May have been cleared above...
+                    {
+                        StringBuffer modifiedDt;
+                        if (!dfsDate.isNull())
+                            dfsDate.getString(modifiedDt);
+                        StringBuffer fileDt;
+                        f->queryDateTime()->getString(fileDt);
+                        if (fileErrorList.find(lfn) == 0)
                         {
-                            case ROXIE_KEY:
-                                fileErrorList.setValue(lfn, "Key");
-                                break;
+                            switch (fileType)
+                            {
+                                case ROXIE_KEY:
+                                    fileErrorList.setValue(lfn, "Key");
+                                    break;
 
-                            case ROXIE_FILE:
-                                fileErrorList.setValue(lfn, "File");
-                                break;
+                                case ROXIE_FILE:
+                                    fileErrorList.setValue(lfn, "File");
+                                    break;
+                            }
                         }
+                        throw MakeStringException(ROXIE_MISMATCH, "Different version of %s already loaded: sizes = %" I64F "d %" I64F "d  Date = %s  %s", lfn, dfsSize, f->getSize(), modifiedDt.str(), fileDt.str());
                     }
-                    throw MakeStringException(ROXIE_MISMATCH, "Different version of %s already loaded: sizes = %" I64F "d %" I64F "d  Date = %s  %s", lfn, dfsSize, f->getSize(), modifiedDt.str(), fileDt.str());
                 }
                 else
                     return f.getClear();

system/jhtree/jhtree.cpp

@@ -59,7 +59,7 @@
 #include "keybuild.hpp"
 #include "layouttrans.hpp"
 
-static CKeyStore *keyStore = NULL;
+static std::atomic<CKeyStore *> keyStore(nullptr);
 static unsigned defaultKeyIndexLimit = 200;
 static CNodeCache *nodeCache = NULL;
 static CriticalSection *initCrit = NULL;
@@ -79,7 +79,7 @@ MODULE_INIT(INIT_PRIORITY_JHTREE_JHTREE)
 MODULE_EXIT()
 {
     delete initCrit;
-    delete keyStore;
+    delete keyStore.load(std::memory_order_relaxed);
     ::Release((CInterface*)nodeCache);
 }
 
@@ -1282,6 +1282,7 @@ void CKeyStore::clearCacheEntry(const char *keyName)
     if (!keyName || !*keyName)
         return;  // nothing to do
 
+    synchronized block(mutex);
     Owned<CKeyIndexMRUCache::CMRUIterator> iter = keyIndexCache.getIterator();
 
     StringArray goers;
@@ -1302,6 +1303,28 @@ void CKeyStore::clearCacheEntry(const char *keyName)
     }
 }
 
+void CKeyStore::clearCacheEntry(const IFileIO *io)
+{
+    synchronized block(mutex);
+    Owned<CKeyIndexMRUCache::CMRUIterator> iter = keyIndexCache.getIterator();
+
+    StringArray goers;
+    ForEach(*iter)
+    {
+        CKeyIndexMapping &mapping = iter->query();
+        IKeyIndex &index = mapping.query();
+        if (!index.IsShared())
+        {
+            if (index.queryFileIO()==io)
+                goers.append(mapping.queryFindString());
+        }
+    }
+    ForEachItemIn(idx, goers)
+    {
+        keyIndexCache.remove(goers.item(idx));
+    }
+}
+
 // CKeyIndex impl.
 
 CKeyIndex::CKeyIndex(int _iD, const char *_name) : name(_name)
@@ -2003,6 +2026,7 @@ public:
     virtual offset_t queryMetadataHead() { return checkOpen().queryMetadataHead(); }
     virtual IPropertyTree * getMetadata() { return checkOpen().getMetadata(); }
     virtual unsigned getNodeSize() { return checkOpen().getNodeSize(); }
+    virtual const IFileIO *queryFileIO() const override { return iFileIO->queryFileIO(); }
 };
 
 extern jhtree_decl IKeyIndex *createKeyIndex(const char *keyfile, unsigned crc, IFileIO &iFileIO, bool isTLK, bool preloadAllowed)
@@ -2038,6 +2062,11 @@ extern jhtree_decl void clearKeyStoreCacheEntry(const char *name)
     queryKeyStore()->clearCacheEntry(name);
 }
 
+extern jhtree_decl void clearKeyStoreCacheEntry(const IFileIO *io)
+{
+    queryKeyStore()->clearCacheEntry(io);
+}
+
 extern jhtree_decl StringBuffer &getIndexMetrics(StringBuffer &ret)
 {
     return queryKeyStore()->getMetrics(ret);

system/jhtree/jhtree.hpp

@@ -80,6 +80,7 @@ interface jhtree_decl IKeyIndex : public IKeyIndexBase
     virtual offset_t queryMetadataHead() = 0;
     virtual IPropertyTree * getMetadata() = 0;
     virtual unsigned getNodeSize() = 0;
+    virtual const IFileIO *queryFileIO() const = 0;
 };
 
 interface IKeyArray : extends IInterface
@@ -103,6 +104,7 @@ interface IReplicatedFile;
 
 extern jhtree_decl void clearKeyStoreCache(bool killAll);
 extern jhtree_decl void clearKeyStoreCacheEntry(const char *name);
+extern jhtree_decl void clearKeyStoreCacheEntry(const IFileIO *io);
 extern jhtree_decl unsigned setKeyIndexCacheSize(unsigned limit);
 extern jhtree_decl void clearNodeCache();
 // these methods return previous values

system/jhtree/jhtree.ipp

@@ -49,6 +49,7 @@ public:
     IKeyIndex *load(const char *fileName, unsigned crc, IReplicatedFile &part, bool isTLK, bool allowPreload);
     void clearCache(bool killAll);
     void clearCacheEntry(const char *name);
+    void clearCacheEntry(const IFileIO *io);
     unsigned setKeyCacheLimit(unsigned limit);
     StringBuffer &getMetrics(StringBuffer &xml);
     void resetMetrics();
@@ -135,6 +136,7 @@ public:
     CMemKeyIndex(int _iD, IMemoryMappedFile *_io, const char *_name, bool _isTLK);
 
     virtual const char *queryFileName() { return name.get(); }
+    virtual const IFileIO *queryFileIO() const override { return nullptr; }
 // INodeLoader impl.
     virtual CJHTreeNode *loadNode(offset_t offset);
 };
@@ -149,6 +151,7 @@ public:
     CDiskKeyIndex(int _iD, IFileIO *_io, const char *_name, bool _isTLK, bool _allowPreload);
 
     virtual const char *queryFileName() { return name.get(); }
+    virtual const IFileIO *queryFileIO() const override { return io; }
 // INodeLoader impl.
     virtual CJHTreeNode *loadNode(offset_t offset);
 };