Use a lock to guard at racing the update of the file object when while is growing

and has multiple readers.

3 files changed, 56 insertions, 43 deletions

diff --git a/searchlib/src/vespa/searchlib/docstore/filechunk.cpp b/searchlib/src/vespa/searchlib/docstore/filechunk.cpp
index 746ec875781..8334e46c8d4 100644
--- a/searchlib/src/vespa/searchlib/docstore/filechunk.cpp
+++ b/searchlib/src/vespa/searchlib/docstore/filechunk.cpp
@@ -10,6 +10,7 @@
 #include <vespa/vespalib/stllike/hash_map.hpp>
 #include <vespa/searchlib/util/filekit.h>
 #include <vespa/vespalib/objects/nbostream.h>
+#include <vespa/fastos/file.h>
 
 #include <vespa/log/log.h>
 LOG_SETUP(".search.filechunk");
diff --git a/searchlib/src/vespa/searchlib/docstore/randreaders.cpp b/searchlib/src/vespa/searchlib/docstore/randreaders.cpp
index 426dbe11300..62f5c96a3a6 100644
--- a/searchlib/src/vespa/searchlib/docstore/randreaders.cpp
+++ b/searchlib/src/vespa/searchlib/docstore/randreaders.cpp
@@ -3,6 +3,7 @@
 #include "randreaders.h"
 #include "summaryexceptions.h"
 #include <vespa/vespalib/data/databuffer.h>
+#include <vespa/fastos/file.h>
 
 #include <vespa/log/log.h>
 LOG_SETUP(".search.docstore.randreaders");
@@ -10,19 +11,19 @@ LOG_SETUP(".search.docstore.randreaders");
 namespace search {
 
 DirectIORandRead::DirectIORandRead(const vespalib::string & fileName)
-    : _file(fileName.c_str()),
+    : _file(std::make_unique<FastOS_File>(fileName.c_str())),
       _alignment(1),
       _granularity(1),
       _maxChunkSize(0x100000)
 {
-    _file.EnableDirectIO();
-    if (_file.OpenReadOnly()) {
-        if (!_file.GetDirectIORestrictions(_alignment, _granularity, _maxChunkSize)) {
+    _file->EnableDirectIO();
+    if (_file->OpenReadOnly()) {
+        if (!_file->GetDirectIORestrictions(_alignment, _granularity, _maxChunkSize)) {
             LOG(debug, "Direct IO setup failed for file %s due to %s",
-                       _file.GetFileName(), _file.getLastErrorString().c_str());
+                       _file->GetFileName(), _file->getLastErrorString().c_str());
         }
     } else {
-        throw SummaryException("Failed opening data file", _file, VESPA_STRLOC);
+        throw SummaryException("Failed opening data file", *_file, VESPA_STRLOC);
     }
 }
 
@@ -31,7 +32,7 @@ DirectIORandRead::read(size_t offset, vespalib::DataBuffer & buffer, size_t sz)
 {
     size_t padBefore(0);
     size_t padAfter(0);
-    bool directio = _file.DirectIOPadding(offset, sz, padBefore, padAfter);
+    bool directio = _file->DirectIOPadding(offset, sz, padBefore, padAfter);
     buffer.clear();
     buffer.ensureFree(padBefore + sz + padAfter + _alignment - 1);
     if (directio) {
@@ -40,7 +41,7 @@ DirectIORandRead::read(size_t offset, vespalib::DataBuffer & buffer, size_t sz)
         buffer.moveDataToDead(unAligned);
     }
     // XXX needs to use pread or file-position-mutex
-    _file.ReadBuf(buffer.getFree(), padBefore + sz + padAfter, offset - padBefore);
+    _file->ReadBuf(buffer.getFree(), padBefore + sz + padAfter, offset - padBefore);
     buffer.moveFreeToData(padBefore + sz);
     buffer.moveDataToDead(padBefore);
     return FSP();
@@ -50,62 +51,71 @@ DirectIORandRead::read(size_t offset, vespalib::DataBuffer & buffer, size_t sz)
 int64_t
 DirectIORandRead::getSize()
 {
-    return _file.GetSize();
+    return _file->GetSize();
 }
 
 
 MMapRandRead::MMapRandRead(const vespalib::string & fileName, int mmapFlags, int fadviseOptions)
-    : _file(fileName.c_str())
+    : _file(std::make_unique<FastOS_File>(fileName.c_str()))
 {
-    _file.enableMemoryMap(mmapFlags);
-    _file.setFAdviseOptions(fadviseOptions);
-    if ( ! _file.OpenReadOnly()) {
-        throw SummaryException("Failed opening data file", _file, VESPA_STRLOC);
+    _file->enableMemoryMap(mmapFlags);
+    _file->setFAdviseOptions(fadviseOptions);
+    if ( ! _file->OpenReadOnly()) {
+        throw SummaryException("Failed opening data file", *_file, VESPA_STRLOC);
     }
 }
 
 
 NormalRandRead::NormalRandRead(const vespalib::string & fileName)
-    : _file(fileName.c_str())
+    : _file(std::make_unique<FastOS_File>(fileName.c_str()))
 {
-    if ( ! _file.OpenReadOnly()) {
-        throw SummaryException("Failed opening data file", _file, VESPA_STRLOC);
+    if ( ! _file->OpenReadOnly()) {
+        throw SummaryException("Failed opening data file", *_file, VESPA_STRLOC);
     }
 }
 
 FileRandRead::FSP
 MMapRandRead::read(size_t offset, vespalib::DataBuffer & buffer, size_t sz)
 {
-    const char *data = static_cast<const char *>(_file.MemoryMapPtr(offset));
+    const char *data = static_cast<const char *>(_file->MemoryMapPtr(offset));
     assert(data != nullptr);
-    assert(_file.MemoryMapPtr(offset+sz-1) != nullptr);
+    assert(_file->MemoryMapPtr(offset+sz-1) != nullptr);
     vespalib::DataBuffer(data, sz).swap(buffer);
     return FSP();
 }
 
 int64_t
-MMapRandRead::getSize()
-{
-    return _file.GetSize();
+MMapRandRead::getSize() {
+    return _file->GetSize();
+}
+
+const void *
+MMapRandRead::getMapping() {
+    return _file->MemoryMapPtr(0);
 }
 
 MMapRandReadDynamic::MMapRandReadDynamic(const vespalib::string &fileName, int mmapFlags, int fadviseOptions)
     : _fileName(fileName),
+      _holder(),
       _mmapFlags(mmapFlags),
-      _fadviseOptions(fadviseOptions)
+      _fadviseOptions(fadviseOptions),
+      _lock()
 {
-    reopen();
+    remap();
 }
 
 void
-MMapRandReadDynamic::reopen()
+MMapRandReadDynamic::remap()
 {
     std::unique_ptr<FastOS_File> file(new FastOS_File(_fileName.c_str()));
     file->enableMemoryMap(_mmapFlags);
     file->setFAdviseOptions(_fadviseOptions);
     if (file->OpenReadOnly()) {
-        _holder.set(file.release());
-        _holder.latch();
+        vespalib::LockGuard guard(_lock);
+        if (file->GetSize() > _holder.get()->GetSize()) {
+            _holder.set(file.release());
+            _holder.latch();
+        }
     } else {
         throw SummaryException("Failed opening data file", *file, VESPA_STRLOC);
     }
@@ -118,7 +128,7 @@ MMapRandReadDynamic::read(size_t offset, vespalib::DataBuffer & buffer, size_t s
     const char * data(static_cast<const char *>(file->MemoryMapPtr(offset)));
     while ((data == nullptr) || (file->MemoryMapPtr(offset+sz-1) == nullptr)) {
         // Must check that both start and end of file is mapped in.
-        reopen();
+        remap();
         file = _holder.get();
         data = static_cast<const char *>(file->MemoryMapPtr(offset));
     }
@@ -136,7 +146,7 @@ NormalRandRead::read(size_t offset, vespalib::DataBuffer & buffer, size_t sz)
 {
     buffer.clear();
     buffer.ensureFree(sz);
-    _file.ReadBuf(buffer.getFree(), sz, offset);
+    _file->ReadBuf(buffer.getFree(), sz, offset);
     buffer.moveFreeToData(sz);
     return FSP();
 }
@@ -144,7 +154,7 @@ NormalRandRead::read(size_t offset, vespalib::DataBuffer & buffer, size_t sz)
 int64_t
 NormalRandRead::getSize()
 {
-    return _file.GetSize();
+    return _file->GetSize();
 }
 
 }
diff --git a/searchlib/src/vespa/searchlib/docstore/randreaders.h b/searchlib/src/vespa/searchlib/docstore/randreaders.h
index fdd8d8c381c..9dee5e306ca 100644
--- a/searchlib/src/vespa/searchlib/docstore/randreaders.h
+++ b/searchlib/src/vespa/searchlib/docstore/randreaders.h
@@ -5,7 +5,8 @@
 #include "randread.h"
 #include <vespa/vespalib/util/ptrholder.h>
 #include <vespa/vespalib/stllike/string.h>
-#include <vespa/fastos/file.h>
+
+class FastOS_FileInterface;
 
 namespace search {
 
@@ -16,10 +17,10 @@ public:
     FSP read(size_t offset, vespalib::DataBuffer & buffer, size_t sz) override;
     int64_t getSize() override;
 private:
-    FastOS_File      _file;
-    size_t           _alignment;
-    size_t           _granularity;
-    size_t           _maxChunkSize;
+    std::unique_ptr<FastOS_FileInterface>  _file;
+    size_t                                 _alignment;
+    size_t                                 _granularity;
+    size_t                                 _maxChunkSize;
 };
 
 class MMapRandRead : public FileRandRead
@@ -28,9 +29,9 @@ public:
     MMapRandRead(const vespalib::string & fileName, int mmapFlags, int fadviseOptions);
     FSP read(size_t offset, vespalib::DataBuffer & buffer, size_t sz) override;
     int64_t getSize() override;
-    const void * getMapping() { return _file.MemoryMapPtr(0); }
+    const void * getMapping();
 private:
-    FastOS_File      _file;
+    std::unique_ptr<FastOS_FileInterface>  _file;
 };
 
 class MMapRandReadDynamic : public FileRandRead
@@ -40,11 +41,12 @@ public:
     FSP read(size_t offset, vespalib::DataBuffer & buffer, size_t sz) override;
     int64_t getSize() override;
 private:
-    void reopen();
-    vespalib::string                 _fileName;
-    vespalib::PtrHolder<FastOS_File> _holder;
-    int                              _mmapFlags;
-    int                              _fadviseOptions;
+    void remap();
+    vespalib::string                          _fileName;
+    vespalib::PtrHolder<FastOS_FileInterface> _holder;
+    int                                       _mmapFlags;
+    int                                       _fadviseOptions;
+    vespalib::Lock                            _lock;
 };
 
 class NormalRandRead : public FileRandRead
@@ -54,7 +56,7 @@ public:
     FSP read(size_t offset, vespalib::DataBuffer & buffer, size_t sz) override;
     int64_t getSize() override;
 private:
-    FastOS_File      _file;
+    std::unique_ptr<FastOS_FileInterface>  _file;
 };
 
 }