You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@qpid.apache.org by kp...@apache.org on 2019/06/07 16:40:12 UTC

[qpid-cpp] branch master updated: QPID-8320: Fix for empty journal file leak when linearstore recovers

This is an automated email from the ASF dual-hosted git repository.

kpvdr pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/qpid-cpp.git


The following commit(s) were added to refs/heads/master by this push:
     new 5d549d8  QPID-8320: Fix for empty journal file leak when linearstore recovers
5d549d8 is described below

commit 5d549d8409f19707f68ca0fbf24ee4e0d3ae8de6
Author: Kim van der Riet <kv...@localhost.localdomain>
AuthorDate: Fri Jun 7 12:39:40 2019 -0400

    QPID-8320: Fix for empty journal file leak when linearstore recovers
---
 management/python/lib/qlslibs/analyze.py         |  2 +-
 src/qpid/linearstore/journal/RecoveryManager.cpp | 32 +++++++++++++++++++-----
 src/qpid/linearstore/journal/RecoveryManager.h   |  2 ++
 src/qpid/linearstore/journal/jcntl.cpp           |  1 +
 4 files changed, 30 insertions(+), 7 deletions(-)

diff --git a/management/python/lib/qlslibs/analyze.py b/management/python/lib/qlslibs/analyze.py
index 8c5de05..b3551e8 100644
--- a/management/python/lib/qlslibs/analyze.py
+++ b/management/python/lib/qlslibs/analyze.py
@@ -505,7 +505,7 @@ class Journal(object):
             self.statistics.filler_record_count += 1
             ok_flag = True
             if self.args.show_all_recs:
-                print '0x%x:%s' % (start_journal_file.file_header.file_num, this_record)
+                print '0x%x:%s' % (start_journal_file.file_header.file_num, this_record.to_rh_string())
         qlslibs.utils.skip(self.current_journal_file.file_header.file_handle, qlslibs.utils.DEFAULT_DBLK_SIZE)
         return ok_flag
     def _handle_enqueue_record(self, enqueue_record, start_journal_file):
diff --git a/src/qpid/linearstore/journal/RecoveryManager.cpp b/src/qpid/linearstore/journal/RecoveryManager.cpp
index 185fa00..be0ed8a 100644
--- a/src/qpid/linearstore/journal/RecoveryManager.cpp
+++ b/src/qpid/linearstore/journal/RecoveryManager.cpp
@@ -298,12 +298,7 @@ void RecoveryManager::recoveryComplete() {
 void RecoveryManager::setLinearFileControllerJournals(lfcAddJournalFileFn fnPtr,
                                                       LinearFileController* lfcPtr) {
     if (journalEmptyFlag_) {
-        if (uninitFileList_.size() > 0) {
-            // TODO: Handle case if uninitFileList_.size() > 1, but this should not happen in normal operation. Here we assume only one item in the list.
-            std::string uninitFile = uninitFileList_.back();
-            uninitFileList_.pop_back();
-            lfcPtr->restoreEmptyFile(uninitFile);
-        }
+        restoreEmptyFile(lfcPtr);
     } else {
         if (initial_fid_ == 0) {
             throw jexception(jerrno::JERR_RCVM_NULLFID, "RecoveryManager", "setLinearFileControllerJournals");
@@ -311,6 +306,9 @@ void RecoveryManager::setLinearFileControllerJournals(lfcAddJournalFileFn fnPtr,
         for (fileNumberMapConstItr_t i = fileNumberMap_.begin(); i != fileNumberMap_.end(); ++i) {
             (lfcPtr->*fnPtr)(i->second->journalFilePtr_, i->second->completedDblkCount_, i->first == initial_fid_);
         }
+        if (lastFileFullFlag_) {
+            restoreEmptyFile(lfcPtr);
+        }
     }
 
     std::ostringstream oss;
@@ -328,6 +326,16 @@ void RecoveryManager::setLinearFileControllerJournals(lfcAddJournalFileFn fnPtr,
     }
 }
 
+void RecoveryManager::restoreEmptyFile(LinearFileController* lfcPtr) {
+    if (uninitFileList_.size() > 0) {
+        std::string uninitFile = uninitFileList_.back();
+        uninitFileList_.pop_back();
+        lfcPtr->restoreEmptyFile(uninitFile);
+        lastFileFullFlag_ = false;
+        endOffset_ = 0;
+    }
+}
+
 std::string RecoveryManager::toString(const std::string& jid, const uint16_t indent) const {
     std::string indentStr(indent, ' ');
     std::ostringstream oss;
@@ -389,6 +397,7 @@ void RecoveryManager::analyzeJournalFileHeaders(efpIdentity_t& efpIdentity) {
             oss << "Journal file " << (*i) << " is corrupted or invalid";
             journalLogRef_.log(JournalLog::LOG_WARN, queueName_, oss.str());
         } else if (hdrEmpty) {
+/* This seems unnecessary, as the data size is encoded in the file_hdr_t::_data_size_kib, even for reset file headers
             // Read symlink, find efp directory name which is efp size in KiB
             // TODO: place this bit into a common function as it is also used in EmptyFilePool.cpp::deleteSymlink()
             char buff[1024];
@@ -406,6 +415,10 @@ void RecoveryManager::analyzeJournalFileHeaders(efpIdentity_t& efpIdentity) {
             uninitFileList_.push_back(*i);
             efpIdentity.pn_ = fileHeader._efp_partition;
             efpIdentity.ds_ = efpDataSize_kib;
+*/
+            efpIdentity.pn_ = fileHeader._efp_partition;
+            efpIdentity.ds_ = fileHeader._data_size_kib;
+            uninitFileList_.push_back(*i);
         } else if (headerQueueName.compare(queueName_) != 0) {
             std::ostringstream oss;
             oss << "Journal file " << (*i) << " belongs to queue \"" << headerQueueName << "\": ignoring";
@@ -947,4 +960,11 @@ void RecoveryManager::removeEmptyFiles(EmptyFilePool* emptyFilePoolPtr) {
     }
 }
 
+void RecoveryManager::removeUninitFiles(EmptyFilePool* emptyFilePoolPtr) {
+    while (uninitFileList_.size() > 0) {
+        emptyFilePoolPtr->returnEmptyFileSymlink(uninitFileList_.back());
+        uninitFileList_.pop_back();
+    };
+}
+
 }}}
diff --git a/src/qpid/linearstore/journal/RecoveryManager.h b/src/qpid/linearstore/journal/RecoveryManager.h
index 55cc6f8..5e23701 100644
--- a/src/qpid/linearstore/journal/RecoveryManager.h
+++ b/src/qpid/linearstore/journal/RecoveryManager.h
@@ -123,6 +123,7 @@ public:
                                  data_tok* const dtokp,
                                  bool ignore_pending_txns);
     void recoveryComplete();
+    void removeUninitFiles(EmptyFilePool* emptyFilePoolPtr);
     void setLinearFileControllerJournals(lfcAddJournalFileFn fnPtr,
                                          LinearFileController* lfcPtr);
     std::string toString(const std::string& jid, const uint16_t indent) const;
@@ -146,6 +147,7 @@ protected:
     bool readFileHeader();
     void readJournalData(char* target, const std::streamsize size);
     void removeEmptyFiles(EmptyFilePool* emptyFilePoolPtr);
+    void restoreEmptyFile(LinearFileController* lfcPtr);
 
     static bool readJournalFileHeader(const std::string& journalFileName,
                                       ::file_hdr_t& fileHeaderRef,
diff --git a/src/qpid/linearstore/journal/jcntl.cpp b/src/qpid/linearstore/journal/jcntl.cpp
index f207b9b..ffa379c 100644
--- a/src/qpid/linearstore/journal/jcntl.cpp
+++ b/src/qpid/linearstore/journal/jcntl.cpp
@@ -122,6 +122,7 @@ jcntl::recover(EmptyFilePoolManager* efpmp,
     _jrnl_log.log(/*LOG_DEBUG*/JournalLog::LOG_INFO, _jid, _recoveryManager.toString(_jid, 5U));
     _linearFileController.initialize(_jdir.dirname(), _emptyFilePoolPtr, _recoveryManager.getHighestFileNumber());
     _recoveryManager.setLinearFileControllerJournals(&qpid::linearstore::journal::LinearFileController::addJournalFile, &_linearFileController);
+    _recoveryManager.removeUninitFiles(_emptyFilePoolPtr);
     if (_recoveryManager.isLastFileFull()) {
         _linearFileController.getNextJournalFile();
     }


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@qpid.apache.org
For additional commands, e-mail: commits-help@qpid.apache.org