diff options
author | Tor Egge <Tor.Egge@broadpark.no> | 2017-08-09 10:50:05 +0200 |
---|---|---|
committer | GitHub <noreply@github.com> | 2017-08-09 10:50:05 +0200 |
commit | c1b3fb8dce2b66857fb528da4d94ac96902e79f6 (patch) | |
tree | 703509e206ac8efdbca5cdf71ca8dea1d3a91d47 | |
parent | ccfecc714b51ff47dff28e9ccd21dec4aeb0e8e3 (diff) | |
parent | 841f58736169a4011d04d70f8dfc91e415ab21d2 (diff) |
Merge pull request #3064 from yahoo/balder/handle-truncate-with-later-write
Avoid opening idxfile to early for writing. Especially as it might ge…
5 files changed, 51 insertions, 40 deletions
diff --git a/fastos/src/vespa/fastos/file.h b/fastos/src/vespa/fastos/file.h index dcc2b401846..8a9de224259 100644 --- a/fastos/src/vespa/fastos/file.h +++ b/fastos/src/vespa/fastos/file.h @@ -514,6 +514,8 @@ public: virtual void EnableSyncWrites(); + bool useSyncWrites() const { return _syncWritesEnabled; } + /** * Set the write chunk size used in WriteBuf. */ diff --git a/searchlib/src/tests/docstore/logdatastore/logdatastore_test.cpp b/searchlib/src/tests/docstore/logdatastore/logdatastore_test.cpp index 407bf3fad98..d41adb2cdef 100644 --- a/searchlib/src/tests/docstore/logdatastore/logdatastore_test.cpp +++ b/searchlib/src/tests/docstore/logdatastore/logdatastore_test.cpp @@ -296,17 +296,21 @@ TEST("testTruncatedIdxFile"){ fileHeaderContext, tlSyncer, NULL); EXPECT_EQUAL(354ul, datastore.lastSyncToken()); } + const char * magic = "mumbo jumbo"; { LogDataStore datastore(executor, "bug-7257706-truncated", config, GrowStrategy(), TuneFileSummary(), fileHeaderContext, tlSyncer, NULL); EXPECT_EQUAL(331ul, datastore.lastSyncToken()); + datastore.write(332, 7, magic, strlen(magic)); + datastore.write(333, 8, magic, strlen(magic)); + datastore.flush(datastore.initFlush(334)); } { LogDataStore datastore(executor, "bug-7257706-truncated", config, GrowStrategy(), TuneFileSummary(), fileHeaderContext, tlSyncer, NULL); - EXPECT_EQUAL(331ul, datastore.lastSyncToken()); + EXPECT_EQUAL(334ul, datastore.lastSyncToken()); } } diff --git a/searchlib/src/vespa/searchlib/docstore/filechunk.cpp b/searchlib/src/vespa/searchlib/docstore/filechunk.cpp index 0f753b4914c..f271e6f320b 100644 --- a/searchlib/src/vespa/searchlib/docstore/filechunk.cpp +++ b/searchlib/src/vespa/searchlib/docstore/filechunk.cpp @@ -194,6 +194,7 @@ FileChunk::updateLidMap(const LockGuard &guard, ISetLid &ds, uint64_t serialNum, } else { throw SummaryException("Open for truncation failed.", toTruncate, VESPA_STRLOC); } + break; } } if ( ! tempVector.empty()) { diff --git a/searchlib/src/vespa/searchlib/docstore/writeablefilechunk.cpp b/searchlib/src/vespa/searchlib/docstore/writeablefilechunk.cpp index edd31ebf12d..c6b93a59ae8 100644 --- a/searchlib/src/vespa/searchlib/docstore/writeablefilechunk.cpp +++ b/searchlib/src/vespa/searchlib/docstore/writeablefilechunk.cpp @@ -94,11 +94,11 @@ WriteableFileChunk(vespalib::ThreadExecutor &executor, _writeLock(), _flushLock(), _dataFile(_dataFileName.c_str()), - _idxFile(_idxFileName.c_str()), _chunkMap(), _pendingChunks(), _pendingIdx(0), _pendingDat(0), + _idxFileSize(0), _currentDiskFootprint(0), _nextChunkId(1), _active(new Chunk(0, Chunk::Config(config.getMaxChunkBytes()))), @@ -116,7 +116,6 @@ WriteableFileChunk(vespalib::ThreadExecutor &executor, } if (tune._write.getWantSyncWrites()) { _dataFile.EnableSyncWrites(); - _idxFile.EnableSyncWrites(); } if (_dataFile.OpenReadWrite()) { readDataHeader(); @@ -130,16 +129,13 @@ WriteableFileChunk(vespalib::ThreadExecutor &executor, _dataFile.GetFileName(), _dataFile.getLastErrorString().c_str()); } } - if (_idxFile.OpenReadWrite()) { - readIdxHeader(); - if (_idxHeaderLen == 0) { - _idxHeaderLen = writeIdxHeader(fileHeaderContext, _docIdLimit, _idxFile); - } - _idxFile.SetPosition(_idxFile.GetSize()); - } else { - _dataFile.Close(); - throw SummaryException("Failed opening idx file", _idxFile, VESPA_STRLOC); + auto idxFile = openIdx(); + readIdxHeader(*idxFile); + if (_idxHeaderLen == 0) { + _idxHeaderLen = writeIdxHeader(fileHeaderContext, _docIdLimit, *idxFile); } + _idxFileSize = idxFile->GetSize(); + idxFile->Sync(); } else { throw SummaryException("Failed opening data file", _dataFile, VESPA_STRLOC); } @@ -147,6 +143,17 @@ WriteableFileChunk(vespalib::ThreadExecutor &executor, updateCurrentDiskFootprint(); } +std::unique_ptr<FastOS_FileInterface> +WriteableFileChunk::openIdx() { + auto file = std::make_unique<FastOS_File>(_idxFileName.c_str()); + if (_dataFile.useSyncWrites()) { + file->EnableSyncWrites(); + } + if ( ! file->OpenReadWrite() ) { + throw SummaryException("Failed opening idx file", *file, VESPA_STRLOC); + } + return file; +} WriteableFileChunk::~WriteableFileChunk() { if (!frozen()) { @@ -162,11 +169,6 @@ WriteableFileChunk::~WriteableFileChunk() assert(false); } } - if (_idxFile.IsOpened()) { - if (! _idxFile.Sync()) { - assert(false); - } - } } size_t @@ -461,7 +463,7 @@ WriteableFileChunk::writeData(const ProcessedChunkQ & chunks, size_t sz) if (wlen != static_cast<ssize_t>(buf.getDataLen())) { throw SummaryException(make_string("Failed writing %ld bytes to dat file. Only %ld written", buf.getDataLen(), wlen), - _idxFile, VESPA_STRLOC); + _dataFile, VESPA_STRLOC); } updateCurrentDiskFootprint(); } @@ -559,7 +561,6 @@ WriteableFileChunk::freeze() _frozen = true; } _dataFile.Close(); - _idxFile.Close(); _bucketMap = BucketDensityComputer(_bucketizer); } } @@ -750,33 +751,32 @@ WriteableFileChunk::readDataHeader() void -WriteableFileChunk::readIdxHeader() +WriteableFileChunk::readIdxHeader(FastOS_FileInterface & idxFile) { - int64_t fSize(_idxFile.GetSize()); + int64_t fSize(idxFile.GetSize()); try { FileHeader h; - _idxHeaderLen = h.readFile(_idxFile); - _idxFile.SetPosition(_idxHeaderLen); + _idxHeaderLen = h.readFile(idxFile); + idxFile.SetPosition(_idxHeaderLen); _docIdLimit = readDocIdLimit(h); } catch (IllegalHeaderException &e) { - _idxFile.SetPosition(0); + idxFile.SetPosition(0); try { - FileHeader::FileReader fr(_idxFile); + FileHeader::FileReader fr(idxFile); uint32_t header2Len = FileHeader::readSize(fr); - if (header2Len <= fSize) + if (header2Len <= fSize) { e.throwSelf(); // header not truncated + } } catch (IllegalHeaderException &e2) { } if (fSize > 0) { // Truncate file (dropping header) if cannot even read // header length, or if header has been truncated. - _idxFile.SetPosition(0); - _idxFile.SetSize(0); - assert(_idxFile.GetSize() == 0); - assert(_idxFile.GetPosition() == 0); - LOG(warning, - "Truncated file chunk index %s due to truncated file header", - _idxFile.GetFileName()); + idxFile.SetPosition(0); + idxFile.SetSize(0); + assert(idxFile.GetSize() == 0); + assert(idxFile.GetPosition() == 0); + LOG(warning, "Truncated file chunk index %s due to truncated file header", idxFile.GetFileName()); } } } @@ -838,7 +838,7 @@ WriteableFileChunk::needFlushPendingChunks(const MonitorGuard & guard, uint64_t void WriteableFileChunk::updateCurrentDiskFootprint() { - _currentDiskFootprint = _idxFile.getSize() + _dataFile.getSize(); + _currentDiskFootprint = _idxFileSize + _dataFile.getSize(); } /* @@ -890,15 +890,18 @@ WriteableFileChunk::unconditionallyFlushPendingChunks(const vespalib::LockGuard } } fastos::TimeStamp timeStamp(fastos::ClockSystem::now()); - ssize_t wlen = _idxFile.Write2(os.c_str(), os.size()); + auto idxFile = openIdx(); + idxFile->SetPosition(idxFile->GetSize()); + ssize_t wlen = idxFile->Write2(os.c_str(), os.size()); updateCurrentDiskFootprint(); if (wlen != static_cast<ssize_t>(os.size())) { - throw SummaryException(make_string("Failed writing %ld bytes to idx file. Only wrote %ld bytes ", os.size(), wlen), _idxFile, VESPA_STRLOC); + throw SummaryException(make_string("Failed writing %ld bytes to idx file. Only wrote %ld bytes ", os.size(), wlen), *idxFile, VESPA_STRLOC); } - if ( ! _idxFile.Sync()) { - throw SummaryException("Failed fsync of idx file", _idxFile, VESPA_STRLOC); + if ( ! idxFile->Sync()) { + throw SummaryException("Failed fsync of idx file", *idxFile, VESPA_STRLOC); } + _idxFileSize = idxFile->GetSize(); if (_lastPersistedSerialNum < lastSerial) { _lastPersistedSerialNum = lastSerial; } diff --git a/searchlib/src/vespa/searchlib/docstore/writeablefilechunk.h b/searchlib/src/vespa/searchlib/docstore/writeablefilechunk.h index a89d88ac93e..86415e0cb40 100644 --- a/searchlib/src/vespa/searchlib/docstore/writeablefilechunk.h +++ b/searchlib/src/vespa/searchlib/docstore/writeablefilechunk.h @@ -90,7 +90,7 @@ private: void restart(uint32_t nextChunkId); ProcessedChunkQ drainQ(); void readDataHeader(); - void readIdxHeader(); + void readIdxHeader(FastOS_FileInterface & idxFile); void writeDataHeader(const common::FileHeaderContext &fileHeaderContext); bool needFlushPendingChunks(uint64_t serialNum, uint64_t datFileLen); bool needFlushPendingChunks(const vespalib::MonitorGuard & guard, uint64_t serialNum, uint64_t datFileLen); @@ -105,6 +105,7 @@ private: void updateChunkInfo(const ProcessedChunkQ & chunks, const ChunkMetaV & cmetaV, size_t sz); void updateCurrentDiskFootprint(); size_t getDiskFootprint(const vespalib::MonitorGuard & guard) const; + std::unique_ptr<FastOS_FileInterface> openIdx(); Config _config; SerialNum _serialNum; @@ -114,13 +115,13 @@ private: vespalib::Lock _writeLock; vespalib::Lock _flushLock; FastOS_File _dataFile; - FastOS_File _idxFile; using ChunkMap = std::map<uint32_t, Chunk::UP>; ChunkMap _chunkMap; using PendingChunks = std::deque<std::shared_ptr<PendingChunk>>; PendingChunks _pendingChunks; uint64_t _pendingIdx; uint64_t _pendingDat; + uint64_t _idxFileSize; uint64_t _currentDiskFootprint; uint32_t _nextChunkId; Chunk::UP _active; |