This is an automated email from the ASF dual-hosted git repository. zwoop pushed a commit to branch master in repository https://gitbox.apache.org/repos/asf/trafficserver.git
commit ba978b66afe6bd3c807fc553c1ead9b7c880806d Author: Leif Hedstrom <[email protected]> AuthorDate: Mon May 20 12:17:50 2019 -0600 Step 1: Removes proxy.config.admin.number_config_bak and code --- doc/admin-guide/files/records.config.en.rst | 4 - lib/perl/lib/Apache/TS/AdminClient.pm | 1 - mgmt/FileManager.cc | 4 +- mgmt/RecordsConfig.cc | 2 - mgmt/Rollback.cc | 146 ++-------------------------- mgmt/Rollback.h | 11 +-- 6 files changed, 18 insertions(+), 150 deletions(-) diff --git a/doc/admin-guide/files/records.config.en.rst b/doc/admin-guide/files/records.config.en.rst index 0c3f0a0..11107d1 100644 --- a/doc/admin-guide/files/records.config.en.rst +++ b/doc/admin-guide/files/records.config.en.rst @@ -496,10 +496,6 @@ Network Local Manager ============= -.. ts:cv:: CONFIG proxy.config.admin.number_config_bak INT 3 - - The maximum number of copies of rolled configuration files to keep. - .. ts:cv:: CONFIG proxy.config.admin.user_id STRING nobody Designates the non-privileged account to run the :program:`traffic_server` diff --git a/lib/perl/lib/Apache/TS/AdminClient.pm b/lib/perl/lib/Apache/TS/AdminClient.pm index 5bc456b..5529d49 100644 --- a/lib/perl/lib/Apache/TS/AdminClient.pm +++ b/lib/perl/lib/Apache/TS/AdminClient.pm @@ -319,7 +319,6 @@ The Apache Traffic Server Administration Manual will explain what these strings proxy.config.task_threads proxy.config.admin.synthetic_port proxy.config.admin.cli_path - proxy.config.admin.number_config_bak proxy.config.admin.user_id proxy.config.alarm.abs_path proxy.config.alarm.bin diff --git a/mgmt/FileManager.cc b/mgmt/FileManager.cc index 9288d92..74ac4c2 100644 --- a/mgmt/FileManager.cc +++ b/mgmt/FileManager.cc @@ -177,7 +177,9 @@ FileManager::rereadConfig() ink_mutex_acquire(&accessLock); for (auto &&it : bindings) { rb = it.second; - if (rb->checkForUserUpdate(rb->isVersioned() ? ROLLBACK_CHECK_AND_UPDATE : ROLLBACK_CHECK_ONLY)) { + // ToDo: rb->isVersions() was always true before, because numberBackups was always >= 1. So ROLLBACK_CHECK_ONLY could not + // happen at all... + if (rb->checkForUserUpdate(ROLLBACK_CHECK_AND_UPDATE)) { changedFiles.push_back(rb); if (rb->isChildRollback()) { if (std::find(parentFileNeedChange.begin(), parentFileNeedChange.end(), rb->getParentRollback()) == diff --git a/mgmt/RecordsConfig.cc b/mgmt/RecordsConfig.cc index eac90b9..9d4a2ad 100644 --- a/mgmt/RecordsConfig.cc +++ b/mgmt/RecordsConfig.cc @@ -244,8 +244,6 @@ static const RecordElement RecordsConfig[] = , {RECT_CONFIG, "proxy.config.admin.autoconf.localhost_only", RECD_INT, "1", RECU_RESTART_TM, RR_NULL, RECC_INT, "[0-1]", RECA_NULL} , - {RECT_CONFIG, "proxy.config.admin.number_config_bak", RECD_INT, "3", RECU_NULL, RR_NULL, RECC_NULL, nullptr, RECA_NULL} - , {RECT_CONFIG, "proxy.config.admin.user_id", RECD_STRING, TS_PKGSYSUSER, RECU_NULL, RR_REQUIRED, RECC_NULL, nullptr, RECA_READ_ONLY} , {RECT_CONFIG, "proxy.config.admin.cli_path", RECD_STRING, "cli", RECU_NULL, RR_NULL, RECC_NULL, nullptr, RECA_NULL} diff --git a/mgmt/Rollback.cc b/mgmt/Rollback.cc index 0f4af40..b6f4ddc 100644 --- a/mgmt/Rollback.cc +++ b/mgmt/Rollback.cc @@ -59,19 +59,11 @@ Rollback::Rollback(const char *fileName_, const char *configName_, bool root_acc parentRollback(parentRollback_), currentVersion(0), fileLastModified(0), - numVersions(0), - numberBackups(0) + numVersions(0) { version_t highestSeen; // the highest backup version ExpandingArray existVer(25, true); // Existing versions struct stat fileInfo; - MgmtInt numBak; - char *alarmMsg; - - // To Test, Read/Write access to the file - int testFD; // For open test - int testErrno; // For open test - ink_assert(fileName_ != nullptr); // parent must not also have a parent @@ -94,135 +86,28 @@ Rollback::Rollback(const char *fileName_, const char *configName_, bool root_acc ink_mutex_init(&fileAccessLock); - if (varIntFromName("proxy.config.admin.number_config_bak", &numBak) == true) { - if (numBak > 1) { - numberBackups = (int)numBak; - } else { - numberBackups = 1; - } - } else { - numberBackups = DEFAULT_BACKUPS; - } - - // If we are not doing backups, bail early. - if ((numberBackups <= 0) || (flags & CONFIG_FLAG_UNVERSIONED)) { + // ToDo: This was really broken before, it used to check if numberBackups <=0, but that could never happen. + if (flags & CONFIG_FLAG_UNVERSIONED) { currentVersion = 0; setLastModifiedTime(); - numberBackups = 0; return; } currentVersion = 0; // Prevent UMR with stat file - highestSeen = findVersions_ml(versionQ); + highestSeen = 0; - // Check to make sure that our configuratio file exists - // - // If we can't find our file, do our best to rollback - // or create an empty one. If that fails, just - // give up + // Check to make sure that our configuration file exists // if (statFile(ACTIVE_VERSION, &fileInfo) < 0) { - // If we can't find an active version because there is not - // one, attempt to rollback to a previous version if one exists - // - // If it does not, create a zero length file to prevent total havoc - // - if (errno == ENOENT) { - bool needZeroLength; - mgmt_log("[RollBack::Rollback] Missing Configuration File: %s\n", fileName); - - if (highestSeen > 0) { - char *highestSeenStr = createPathStr(highestSeen); - char *activeVerStr = createPathStr(ACTIVE_VERSION); - - if (rename(highestSeenStr, activeVerStr) < 0) { - mgmt_log("[RollBack::Rollback] Automatic Rollback to prior version failed for %s : %s\n", fileName, strerror(errno)); - needZeroLength = true; - } else { - mgmt_log("[RollBack::Rollback] Automatic Rollback to version succeeded for %s\n", fileName, strerror(errno)); - needZeroLength = false; - highestSeen--; - // Since we've made the highestVersion active - // remove it from the backup version q - versionQ.remove(versionQ.tail); - } - ats_free(highestSeenStr); - ats_free(activeVerStr); - } else { - needZeroLength = true; - } - - if (needZeroLength == true) { - int fd = openFile(ACTIVE_VERSION, O_RDWR | O_CREAT); - if (fd >= 0) { - alarmMsg = (char *)ats_malloc(2048); - snprintf(alarmMsg, 2048, "Created zero length place holder for config file %s", fileName); - mgmt_log("[RollBack::Rollback] %s\n", alarmMsg); - lmgmt->alarm_keeper->signalAlarm(MGMT_ALARM_CONFIG_UPDATE_FAILED, alarmMsg); - ats_free(alarmMsg); - closeFile(fd, true); - } else { - mgmt_fatal(0, "[RollBack::Rollback] Unable to find configuration file %s.\n\tCreation of a placeholder failed : %s\n", - fileName, strerror(errno)); - } - } + // If we can't find an active version because there is none we have a hard failure. + mgmt_fatal(0, "[RollBack::Rollback] Unable to find configuration file %s.\n\tStat failed : %s\n", fileName, strerror(errno)); - currentVersion = highestSeen + 1; - setLastModifiedTime(); - } else { - // If is there but we can not stat it, it is unusable to manager - // probably due to permissions problems. Bail! - mgmt_fatal(0, "[RollBack::Rollback] Unable to find configuration file %s.\n\tStat failed : %s\n", fileName, strerror(errno)); - } } else { fileLastModified = TS_ARCHIVE_STAT_MTIME(fileInfo); currentVersion = highestSeen + 1; - // Make sure that we have a backup of the file - if (highestSeen == 0) { - TextBuffer *version0 = nullptr; - char failStr[] = "[Rollback::Rollback] Automatic Roll of Version 1 failed: %s"; - if (getVersion_ml(ACTIVE_VERSION, &version0) != OK_ROLLBACK) { - mgmt_log(failStr, fileName); - } else { - if (forceUpdate_ml(version0) != OK_ROLLBACK) { - mgmt_log(failStr, fileName); - } - } - if (version0 != nullptr) { - delete version0; - } - } - Debug("rollback", "[Rollback::Rollback] Current Version of %s is %d", fileName, currentVersion); } - - // Now that we'll got every thing set up, try opening - // the file to make sure that we will actually be able to - // read and write it - testFD = openFile(ACTIVE_VERSION, O_RDWR, &testErrno); - if (testFD < 0) { - // We failed to open read-write - alarmMsg = (char *)ats_malloc(2048); - testFD = openFile(ACTIVE_VERSION, O_RDONLY, &testErrno); - - if (testFD < 0) { - // We are unable to either read or write the file - snprintf(alarmMsg, 2048, "Unable to read or write config file"); - mgmt_log("[Rollback::Rollback] %s %s: %s\n", alarmMsg, fileName, strerror(errno)); - lmgmt->alarm_keeper->signalAlarm(MGMT_ALARM_CONFIG_UPDATE_FAILED, alarmMsg); - - } else { - // Read is OK and write fails - snprintf(alarmMsg, 2048, "Config file is read-only"); - mgmt_log("[Rollback::Rollback] %s : %s\n", alarmMsg, fileName); - lmgmt->alarm_keeper->signalAlarm(MGMT_ALARM_CONFIG_UPDATE_FAILED, alarmMsg); - closeFile(testFD, false); - } - ats_free(alarmMsg); - } else { - closeFile(testFD, true); - } } Rollback::~Rollback() @@ -376,7 +261,6 @@ Rollback::internalUpdate(TextBuffer *buf, version_t newVersion, bool notifyChang ssize_t writeBytes; int diskFD; int ret; - versionInfo *toRemove; versionInfo *newBak; bool failedLink = false; char *alarmMsg = nullptr; @@ -448,19 +332,9 @@ Rollback::internalUpdate(TextBuffer *buf, version_t newVersion, bool notifyChang } setLastModifiedTime(); - // Check to see if we need to delete an excess backup versions - // - // We subtract one from numVersions to exclude the active - // copy we just created. If we subtracted two, but left - // the toRemove calculation the same, version one would - // never get deleted - // - if (numVersions >= this->numberBackups && failedLink == false) { - toRemove = versionQ.head; - ink_release_assert(toRemove != nullptr); - ink_assert((toRemove->version) < this->currentVersion); - removeVersion_ml(toRemove->version); - } + + // ToDo: We used to call removeVersion_ml() here. + // If we created a backup version add it to the // List of backup Versions if (failedLink == false) { diff --git a/mgmt/Rollback.h b/mgmt/Rollback.h index bdcaebd..3186faf 100644 --- a/mgmt/Rollback.h +++ b/mgmt/Rollback.h @@ -193,31 +193,31 @@ public: { return fileBaseName; } + const char * getFileName() const { return fileName; } + const char * getConfigName() const { return configName; } + bool isChildRollback() const { return parentRollback != nullptr; } + Rollback * getParentRollback() const { return parentRollback; } - bool - isVersioned() const - { - return numberBackups > 0; - } + bool rootAccessNeeded() const { @@ -246,7 +246,6 @@ private: version_t currentVersion; time_t fileLastModified; int numVersions; - int numberBackups; Queue<versionInfo> versionQ; // stores the backup version info };
