----- Original Message -----
> Updated Branches:
> refs/heads/master c78f95423 -> 7fd40eba2
>
>
> TS-2471: records.config becomes empty when the disk full
>
> In lib/records/RecLocal.cc and lib/records/RecProcess.cc, records.config
> may become empty when the disk is full because we write records.config
> directly. We should write to a temporary file, then rename it to
> records.config.
>
>
> Project: http://git-wip-us.apache.org/repos/asf/trafficserver/repo
> Commit: http://git-wip-us.apache.org/repos/asf/trafficserver/commit/7fd40eba
> Tree: http://git-wip-us.apache.org/repos/asf/trafficserver/tree/7fd40eba
> Diff: http://git-wip-us.apache.org/repos/asf/trafficserver/diff/7fd40eba
>
> Branch: refs/heads/master
> Commit: 7fd40eba2157a3d709a9f254cad156ab44fbb465
> Parents: c78f954
> Author: Yu Qing <zhuangy...@taobao.com>
> Authored: Tue Jan 7 18:35:54 2014 +0800
> Committer: James Peach <jpe...@apache.org>
> Committed: Wed Jan 8 09:44:46 2014 -0800
>
> ----------------------------------------------------------------------
> CHANGES | 3 ++
> lib/records/P_RecCore.cc | 79 ++++++++++++++++++++++++++++++++++++++++++
> lib/records/P_RecCore.h | 1 +
> lib/records/P_RecFile.h | 3 +-
> lib/records/RecFile.cc | 13 +++++--
> lib/records/RecLocal.cc | 12 ++-----
> lib/records/RecMessage.cc | 1 +
> lib/records/RecProcess.cc | 6 +---
> mgmt/Rollback.cc | 43 +++++++++++++----------
> mgmt/Rollback.h | 2 +-
> 10 files changed, 125 insertions(+), 38 deletions(-)
> ----------------------------------------------------------------------
>
>
> http://git-wip-us.apache.org/repos/asf/trafficserver/blob/7fd40eba/CHANGES
> ----------------------------------------------------------------------
> diff --git a/CHANGES b/CHANGES
> index 3f6442d..2835c24 100644
> --- a/CHANGES
> +++ b/CHANGES
> @@ -1,6 +1,9 @@
> -*- coding: utf-8
> -*-
> Changes with Apache Traffic Server 4.2.0
>
> + *) [TS-2471] Writing records.config can fail when the disk is full.
> + Author: Yu Qing <zhuangy...@taobao.com>
> +
> *) [TS-2479] Don't output orphan log after failover sucessfully.
>
> *) [TS-2370] SSL proxy.config.ssl.server.honor_cipher_order is backwards.
>
> http://git-wip-us.apache.org/repos/asf/trafficserver/blob/7fd40eba/lib/records/P_RecCore.cc
> ----------------------------------------------------------------------
> diff --git a/lib/records/P_RecCore.cc b/lib/records/P_RecCore.cc
> index 6f02a20..f176192 100644
> --- a/lib/records/P_RecCore.cc
> +++ b/lib/records/P_RecCore.cc
> @@ -925,3 +925,82 @@ RecSetSyncRequired(char *name, bool lock)
>
> return err;
> }
> +
> +int RecWriteConfigFile(textBuffer *tb)
> +{
> +#define TMP_FILENAME_EXT_STR ".tmp"
> +#define TMP_FILENAME_EXT_LEN (sizeof(TMP_FILENAME_EXT_STR) - 1)
Why not use mkstemp(3) here?
> +
> + int nbytes;
> + int filename_len;
> + int tmp_filename_len;
> + int result;
> + char buff[1024];
> + char *tmp_filename;
> +
> + filename_len = strlen(g_rec_config_fpath);
> + tmp_filename_len = filename_len + TMP_FILENAME_EXT_LEN;
> + if (tmp_filename_len < (int)sizeof(buff)) {
> + tmp_filename = buff;
> + } else {
> + tmp_filename = (char *)ats_malloc(tmp_filename_len + 1);
> + }
> + sprintf(tmp_filename, "%s%s", g_rec_config_fpath, TMP_FILENAME_EXT_STR);
> +
> + RecDebug(DL_Note, "Writing '%s'", g_rec_config_fpath);
> +
> + RecHandle h_file = RecFileOpenW(tmp_filename);
> + do {
> + if (h_file == REC_HANDLE_INVALID) {
> + RecLog(DL_Warning, "open file: %s to write fail, errno: %d, error
> info: %s",
> + tmp_filename, errno, strerror(errno));
> + result = REC_ERR_FAIL;
> + break;
> + }
> +
> + if (RecFileWrite(h_file, tb->bufPtr(), tb->spaceUsed(), &nbytes) !=
> REC_ERR_OKAY) {
> + RecLog(DL_Warning, "write to file: %s fail, errno: %d, error info:
> %s",
> + tmp_filename, errno, strerror(errno));
> + result = REC_ERR_FAIL;
> + break;
> + }
> +
> + if (nbytes != tb->spaceUsed()) {
> + RecLog(DL_Warning, "write to file: %s fail, disk maybe full",
> tmp_filename);
> + result = REC_ERR_FAIL;
> + break;
> + }
> +
> + if (RecFileSync(h_file) != REC_ERR_OKAY) {
> + RecLog(DL_Warning, "fsync file: %s fail, errno: %d, error info: %s",
> + tmp_filename, errno, strerror(errno));
> + result = REC_ERR_FAIL;
> + break;
> + }
> + if (RecFileClose(h_file) != REC_ERR_OKAY) {
> + RecLog(DL_Warning, "close file: %s fail, errno: %d, error info: %s",
> + tmp_filename, errno, strerror(errno));
> + result = REC_ERR_FAIL;
> + break;
> + }
> + h_file = REC_HANDLE_INVALID;
> +
> + if (rename(tmp_filename, g_rec_config_fpath) != 0) {
> + RecLog(DL_Warning, "rename file %s to %s fail, errno: %d, error info:
> %s",
> + tmp_filename, g_rec_config_fpath, errno, strerror(errno));
> + result = REC_ERR_FAIL;
> + break;
> + }
> +
> + result = REC_ERR_OKAY;
> + } while (0);
> +
> + if (h_file != REC_HANDLE_INVALID) {
> + RecFileClose(h_file);
> + }
> + if (tmp_filename != buff) {
> + ats_free(tmp_filename);
> + }
> + return result;
> +}
> +
>
> http://git-wip-us.apache.org/repos/asf/trafficserver/blob/7fd40eba/lib/records/P_RecCore.h
> ----------------------------------------------------------------------
> diff --git a/lib/records/P_RecCore.h b/lib/records/P_RecCore.h
> index 375f526..f0335b9 100644
> --- a/lib/records/P_RecCore.h
> +++ b/lib/records/P_RecCore.h
> @@ -87,6 +87,7 @@ int RecGetRecord_Xmalloc(const char *name, RecDataT
> data_type, RecData * data, b
> int RecReadStatsFile();
> int RecSyncStatsFile();
> int RecReadConfigFile(bool inc_version);
> +int RecWriteConfigFile(textBuffer *tb);
> int RecSyncConfigToTB(textBuffer * tb, bool *inc_version = NULL);
>
> //-------------------------------------------------------------------------
>
> http://git-wip-us.apache.org/repos/asf/trafficserver/blob/7fd40eba/lib/records/P_RecFile.h
> ----------------------------------------------------------------------
> diff --git a/lib/records/P_RecFile.h b/lib/records/P_RecFile.h
> index 48ce70d..25b0a49 100644
> --- a/lib/records/P_RecFile.h
> +++ b/lib/records/P_RecFile.h
> @@ -37,11 +37,12 @@ typedef int RecHandle;
>
> RecHandle RecFileOpenR(const char *file);
> RecHandle RecFileOpenW(const char *file);
> -void RecFileClose(RecHandle h_file);
> +int RecFileClose(RecHandle h_file);
> int RecFileRead(RecHandle h_file, char *buf, int size, int *bytes_read);
> int RecFileWrite(RecHandle h_file, char *buf, int size, int *bytes_written);
> int RecFileGetSize(RecHandle h_file);
> int RecFileExists(const char *file);
> +int RecFileSync(RecHandle h_file);
>
> //-------------------------------------------------------------------------
> // RecPipe
>
> http://git-wip-us.apache.org/repos/asf/trafficserver/blob/7fd40eba/lib/records/RecFile.cc
> ----------------------------------------------------------------------
> diff --git a/lib/records/RecFile.cc b/lib/records/RecFile.cc
> index 2a0b52f..1bb9750 100644
> --- a/lib/records/RecFile.cc
> +++ b/lib/records/RecFile.cc
> @@ -54,13 +54,22 @@ RecFileOpenW(const char *file)
> }
>
> //-------------------------------------------------------------------------
> +// RecFileSync
> +//-------------------------------------------------------------------------
> +
> +int RecFileSync(RecHandle h_file)
> +{
> + return (fsync(h_file) == 0) ? REC_ERR_OKAY : REC_ERR_FAIL;
> +}
> +
> +//-------------------------------------------------------------------------
> // RecFileClose
> //-------------------------------------------------------------------------
>
> -void
> +int
> RecFileClose(RecHandle h_file)
> {
> - close(h_file);
> + return (close(h_file) == 0) ? REC_ERR_OKAY : REC_ERR_FAIL;
> }
>
> //-------------------------------------------------------------------------
>
> http://git-wip-us.apache.org/repos/asf/trafficserver/blob/7fd40eba/lib/records/RecLocal.cc
> ----------------------------------------------------------------------
> diff --git a/lib/records/RecLocal.cc b/lib/records/RecLocal.cc
> index 9502bb9..87622f4 100644
> --- a/lib/records/RecLocal.cc
> +++ b/lib/records/RecLocal.cc
> @@ -72,11 +72,7 @@ sync_thr(void *data)
> send_push_message();
> RecSyncStatsFile();
> if (RecSyncConfigToTB(&tb) == REC_ERR_OKAY) {
> - int nbytes;
> - RecDebug(DL_Note, "Writing '%s'", g_rec_config_fpath);
> - RecHandle h_file = RecFileOpenW(g_rec_config_fpath);
> - RecFileWrite(h_file, tb.bufPtr(), tb.spaceUsed(), &nbytes);
> - RecFileClose(h_file);
> + RecWriteConfigFile(&tb);
> }
> usleep(REC_REMOTE_SYNC_INTERVAL_MS * 1000);
> }
> @@ -124,11 +120,7 @@ sync_thr(void *data)
> rb = NULL;
> }
> if (!written) {
> - int nbytes;
> - RecDebug(DL_Note, "Writing '%s'", g_rec_config_fpath);
> - RecHandle h_file = RecFileOpenW(g_rec_config_fpath);
> - RecFileWrite(h_file, tb->bufPtr(), tb->spaceUsed(), &nbytes);
> - RecFileClose(h_file);
> + RecWriteConfigFile(tb);
> if (rb != NULL) {
> rb->setLastModifiedTime();
> }
>
> http://git-wip-us.apache.org/repos/asf/trafficserver/blob/7fd40eba/lib/records/RecMessage.cc
> ----------------------------------------------------------------------
> diff --git a/lib/records/RecMessage.cc b/lib/records/RecMessage.cc
> index bec22be..fd25d4d 100644
> --- a/lib/records/RecMessage.cc
> +++ b/lib/records/RecMessage.cc
> @@ -552,6 +552,7 @@ RecMessageWriteToDisk(RecMessage *msg, const char *fpath)
> msg_size = sizeof(RecMessageHdr) + (msg->o_write - msg->o_start);
> if ((h_file = RecFileOpenW(fpath)) != REC_HANDLE_INVALID) {
> if (RecFileWrite(h_file, (char *) msg, msg_size, &bytes_written) ==
> REC_ERR_FAIL) {
> + RecFileClose(h_file);
> return REC_ERR_FAIL;
> }
> RecFileClose(h_file);
>
> http://git-wip-us.apache.org/repos/asf/trafficserver/blob/7fd40eba/lib/records/RecProcess.cc
> ----------------------------------------------------------------------
> diff --git a/lib/records/RecProcess.cc b/lib/records/RecProcess.cc
> index 75b5829..5dc7392 100644
> --- a/lib/records/RecProcess.cc
> +++ b/lib/records/RecProcess.cc
> @@ -392,11 +392,7 @@ struct sync_cont: public Continuation
> send_push_message();
> RecSyncStatsFile();
> if (RecSyncConfigToTB(m_tb) == REC_ERR_OKAY) {
> - int nbytes;
> - RecDebug(DL_Note, "Writing '%s'", g_rec_config_fpath);
> - RecHandle h_file = RecFileOpenW(g_rec_config_fpath);
> - RecFileWrite(h_file, m_tb->bufPtr(), m_tb->spaceUsed(), &nbytes);
> - RecFileClose(h_file);
> + RecWriteConfigFile(m_tb);
> }
> Debug("statsproc", "sync_cont() processed");
>
>
> http://git-wip-us.apache.org/repos/asf/trafficserver/blob/7fd40eba/mgmt/Rollback.cc
> ----------------------------------------------------------------------
> diff --git a/mgmt/Rollback.cc b/mgmt/Rollback.cc
> index 6c219e3..4489a18 100644
> --- a/mgmt/Rollback.cc
> +++ b/mgmt/Rollback.cc
> @@ -140,7 +140,7 @@ Rollback::Rollback(const char *baseFileName, bool
> root_access_needed_)
> mgmt_log(stderr, "[RollBack::Rollback] %s\n", alarmMsg);
> lmgmt->alarm_keeper->signalAlarm(MGMT_ALARM_CONFIG_UPDATE_FAILED,
> alarmMsg);
> ats_free(alarmMsg);
> - closeFile(fd);
> + closeFile(fd, true);
> } else {
> mgmt_fatal(stderr, 0,
> "[RollBack::Rollback] Unable to find configuration file
> %s.\n\tCreation of a placeholder failed : %s\n",
> @@ -201,11 +201,11 @@ Rollback::Rollback(const char *baseFileName, bool
> root_access_needed_)
> snprintf(alarmMsg, 2048, "Config file is read-only");
> mgmt_log(stderr, "[Rollback::Rollback] %s : %s\n", alarmMsg,
> fileName);
> lmgmt->alarm_keeper->signalAlarm(MGMT_ALARM_CONFIG_UPDATE_FAILED,
> alarmMsg);
> - closeFile(testFD);
> + closeFile(testFD, false);
> }
> ats_free(alarmMsg);
> } else {
> - closeFile(testFD);
> + closeFile(testFD, true);
> }
> }
>
> @@ -338,8 +338,9 @@ Rollback::openFile(version_t version, int oflags, int
> *errnoPtr)
> }
> mgmt_log(stderr, "[Rollback::openFile] Open of %s failed: %s\n",
> fileName, strerror(errno));
> }
> -
> - fcntl(fd, F_SETFD, 1);
> + else {
> + fcntl(fd, F_SETFD, 1);
> + }
>
> delete[]filePath;
>
> @@ -347,18 +348,21 @@ Rollback::openFile(version_t version, int oflags, int
> *errnoPtr)
> }
>
> int
> -Rollback::closeFile(int fd)
> +Rollback::closeFile(int fd, bool callSync)
> {
> - if (fsync(fd) == -1) {
> - // INKqa11574: SGI fsync will return EBADF error if the file was
> - // open w/o write access (e.g. read-only). Do not print an error
> - // if we get this errno back -- just in case this fd was open
> - // O_RDONLY.
> - if (errno != EBADF) {
> - mgmt_log(stderr, "[Rollback::closeFile] fsync failed for file '%s'
> (%d: %s)\n", fileName, errno, strerror(errno));
> - }
> + int result = 0;
> + if (callSync && fsync(fd) < 0) {
> + result = -1;
> + mgmt_log(stderr, "[Rollback::closeFile] fsync failed for file '%s' (%d:
> %s)\n", fileName, errno, strerror(errno));
> }
> - return (close(fd));
> +
> + if (result == 0) {
> + result = close(fd);
> + }
> + else {
> + close(fd);
> + }
> + return result;
> }
>
>
> @@ -422,6 +426,7 @@ Rollback::internalUpdate(textBuffer * buf, version_t
> newVersion, bool notifyChan
> char *nextVersion;
> int writeBytes;
> int diskFD;
> + int ret;
> versionInfo *toRemove;
> versionInfo *newBak;
> bool failedLink = false;
> @@ -465,9 +470,9 @@ Rollback::internalUpdate(textBuffer * buf, version_t
> newVersion, bool notifyChan
> }
> // Write the buffer into the new configuration file
> writeBytes = write(diskFD, buf->bufPtr(), buf->spaceUsed());
> - closeFile(diskFD);
> - if (writeBytes != buf->spaceUsed()) {
> - mgmt_log(stderr, "[Rollback::intrernalUpdate] Unable to write new
> version of %s : %s\n", fileName, strerror(errno));
> + ret = closeFile(diskFD, true);
> + if ((ret < 0) || (writeBytes != buf->spaceUsed())) {
> + mgmt_log(stderr, "[Rollback::internalUpdate] Unable to write new version
> of %s : %s\n", fileName, strerror(errno));
> returnCode = SYS_CALL_ERROR_ROLLBACK;
> goto UPDATE_CLEANUP;
> }
> @@ -632,7 +637,7 @@ Rollback::getVersion_ml(version_t version, textBuffer **
> buffer)
> GET_CLEANUP:
>
> if (diskFD >= 0) {
> - closeFile(diskFD);
> + closeFile(diskFD, false);
> }
>
> return returnCode;
>
> http://git-wip-us.apache.org/repos/asf/trafficserver/blob/7fd40eba/mgmt/Rollback.h
> ----------------------------------------------------------------------
> diff --git a/mgmt/Rollback.h b/mgmt/Rollback.h
> index e3025a6..7b877a0 100644
> --- a/mgmt/Rollback.h
> +++ b/mgmt/Rollback.h
> @@ -203,7 +203,7 @@ public:
> private:
> Rollback(const Rollback &);
> int openFile(version_t version, int oflags, int *errnoPtr = NULL);
> - int closeFile(int fd);
> + int closeFile(int fd, bool callSync);
> int statFile(version_t version, struct stat *buf);
> char *createPathStr(version_t version);
> RollBackCodes internalUpdate(textBuffer * buf, version_t newVersion, bool
> notifyChange = true, bool incVersion = true);
>
>
--
Igor Galić
Tel: +43 (0) 664 886 22 883
Mail: i.ga...@brainsware.org
URL: http://brainsware.org/
GPG: 8716 7A9F 989B ABD5 100F 4008 F266 55D6 2998 1641
