features/changelog: Consider only changelog on/off as changelog breakage

Earlier, both chagelog on/off and brick restart were considered
to be changelog breakage and treated as changelog not being
continuous. As a result, new HTIME.TSTAMP file was created on
both the above cases. Now the change is made such that only
on changelog enable/disable, the changelog is considered to be
discontinuous. New HTIME.TSTAMP file is not created on brick
restart, the changelogs files are appended to last HTIME.TSTAMP
file.

Treating changelog as continuous in above scenario is important
as changelog history API will fail otherwise. It can successfully
get changes between start and end timestamps only when changelog
is continuous (Changelogs in single HTIME.TSTAMP file are treated
as continuous). Without this change, changelog history API would
fail, and it would become necessary to fallback to other mechanisms
like xsync FSCrawl in case geo-rep to detect changes in this time
window. But Xsync FSCrawl would not be applicable to other
consumers like glusterfind.

Rationale:
1. In plain distributed volume, if brick goes down, no I/O can
   happen onto the brick. Hence changelog is intact with data
   on disk.
2. In distributed replicate volume, if brick goes down, since
   self-heal traffic is captured in changelog. Eventually,
   I/O happened whend brick down is captured in changelog.

Change-Id: I2eb66efe6ee9a9228fb1fcb38d6e7696b9559d5b
BUG: 1211327
Signed-off-by: Kotresh HR <khiremat@redhat.com>
Reviewed-on: http://review.gluster.org/10222
Reviewed-by: Venky Shankar <vshankar@redhat.com>
Tested-by: Venky Shankar <vshankar@redhat.com>
Tested-by: Gluster Build System <jenkins@build.gluster.com>
Tested-by: NetBSD Build System

7 files changed, 268 insertions, 11 deletions

diff --git a/tests/bugs/changelog/bug-1211327.t b/tests/bugs/changelog/bug-1211327.t
new file mode 100644
index 00000000000..19d6e76ecab
--- /dev/null
+++ b/tests/bugs/changelog/bug-1211327.t
@@ -0,0 +1,48 @@
+#!/bin/bash
+
+#Testcase:
+#On brick restart, new HTIME.TSTAMP file should not be created.
+#But on changelog disable/enable HTIME.TSTAMP should be created.
+
+. $(dirname $0)/../../include.rc
+. $(dirname $0)/../../volume.rc
+. $(dirname $0)/../../changelog.rc
+cleanup;
+
+## Start and create a volume
+TEST glusterd;
+TEST pidof glusterd;
+TEST $CLI volume create $V0 $H0:$B0/$V0"1";
+
+## Verify volume is is created
+EXPECT "$V0" volinfo_field $V0 'Volume Name';
+EXPECT 'Created' volinfo_field $V0 'Status';
+
+## Start volume and verify
+TEST $CLI volume start $V0;
+EXPECT 'Started' volinfo_field $V0 'Status';
+
+TEST $CLI volume set $V0 changelog.changelog on;
+##Let changelog init complete before killing gluster processes
+sleep 1
+
+TEST killall_gluster;
+sleep 1
+EXPECT 0 online_brick_count;
+
+TEST glusterd;
+TEST pidof glusterd;
+##Let the brick processes starts
+sleep 1;
+
+EXPECT 1 online_brick_count;
+
+##On brick restart only one HTIME should be found.
+EXPECT 1 count_htime_files;
+
+##On changelog disable/enable, new HTIME should be created.
+TEST $CLI volume set $V0 changelog.changelog off;
+TEST $CLI volume set $V0 changelog.changelog on;
+EXPECT 2 count_htime_files;
+
+cleanup;
diff --git a/tests/changelog.rc b/tests/changelog.rc
new file mode 100644
index 00000000000..e3b040226ec
--- /dev/null
+++ b/tests/changelog.rc
@@ -0,0 +1,4 @@
+
+function count_htime_files {
+    ls -l $B0/$V0"1"/.glusterfs/changelogs/htime/ | grep HTIME | wc -l
+}
diff --git a/tests/volume.rc b/tests/volume.rc
index 6c0e6abea83..b0211cbe2fc 100644
--- a/tests/volume.rc
+++ b/tests/volume.rc
@@ -410,6 +410,11 @@ function volume_exists() {
 	fi
 }
 
+function killall_gluster() {
+        pkill gluster
+        sleep 1
+}
+
 function afr_get_index_count {
         local brick=$1
         ls $1/.glusterfs/indices/xattrop | grep -v xattrop | wc -l
diff --git a/xlators/features/changelog/src/changelog-helpers.c b/xlators/features/changelog/src/changelog-helpers.c
index aef847c037b..585214df635 100644
--- a/xlators/features/changelog/src/changelog-helpers.c
+++ b/xlators/features/changelog/src/changelog-helpers.c
@@ -422,17 +422,177 @@ changelog_rollover_changelog (xlator_t *this,
         return ret;
 }
 
-/* Returns 0 on successful creation of htime file
+int
+filter_cur_par_dirs (const struct dirent *entry)
+{
+        if (entry == NULL)
+                return 0;
+
+        if ((strcmp(entry->d_name, ".") == 0) ||
+            (strcmp(entry->d_name, "..") == 0))
+                return 0;
+        else
+                return 1;
+}
+
+/*
+ * find_current_htime:
+ *       It finds the latest htime file and sets the HTIME_CURRENT
+ *       xattr.
+ *       RETURN VALUE:
+ *           -1 : Error
+ *           ret: Number of directory entries;
+ */
+
+int
+find_current_htime (int ht_dir_fd, const char *ht_dir_path, char *ht_file_bname)
+{
+        struct dirent       **namelist = NULL;
+        int                   ret      = 0;
+        int                   cnt      = 0;
+        int                   i        = 0;
+        xlator_t             *this     = NULL;
+
+        this = THIS;
+        GF_ASSERT (this);
+        GF_ASSERT (ht_dir_path);
+
+        cnt = scandir (ht_dir_path, &namelist, filter_cur_par_dirs, alphasort);
+        if (cnt < 0) {
+                gf_log (this->name, GF_LOG_ERROR,
+                        "scandir failed: %s", strerror (errno));
+        } else if (cnt > 0) {
+                strncpy (ht_file_bname, namelist[cnt - 1]->d_name, NAME_MAX);
+                ht_file_bname[NAME_MAX - 1] = 0;
+
+                if (sys_fsetxattr (ht_dir_fd, HTIME_CURRENT, ht_file_bname,
+                    strlen (ht_file_bname), 0)) {
+                        gf_log (this->name, GF_LOG_ERROR, "fsetxattr failed:"
+                                " HTIME_CURRENT: %s", strerror (errno));
+                        ret = -1;
+                        goto out;
+                }
+
+                if (fsync (ht_dir_fd) < 0) {
+                        gf_log (this->name, GF_LOG_ERROR,
+                                "fsync failed (reason: %s)", strerror (errno));
+                        ret = -1;
+                        goto out;
+                }
+        }
+
+ out:
+        for (i = 0; i < cnt; i++)
+                free (namelist[i]);
+        free (namelist);
+
+        if (ret)
+                cnt = ret;
+
+        return cnt;
+}
+
+/* Returns 0 on successful open of htime file
  * returns -1 on failure or error
  */
 int
 htime_open (xlator_t *this,
-            changelog_priv_t * priv, unsigned long ts)
+              changelog_priv_t *priv, unsigned long ts)
 {
-        int fd                          = -1;
+        int ht_file_fd                  = -1;
+        int ht_dir_fd                   = -1;
         int ret                         = 0;
+        int cnt                         = 0;
         char ht_dir_path[PATH_MAX]      = {0,};
         char ht_file_path[PATH_MAX]     = {0,};
+        char ht_file_bname[NAME_MAX]    = {0,};
+        char x_value[NAME_MAX]          = {0,};
+        int flags                       = 0;
+        unsigned long min_ts            = 0;
+        unsigned long max_ts            = 0;
+        unsigned long total             = 0;
+        ssize_t size                    = 0;
+
+        CHANGELOG_FILL_HTIME_DIR(priv->changelog_dir, ht_dir_path);
+
+        /* Open htime directory to get HTIME_CURRENT */
+        ht_dir_fd = open (ht_dir_path, O_RDONLY);
+        if (ht_dir_fd == -1) {
+                gf_log (this->name, GF_LOG_ERROR, "open failed: %s : %s",
+                        ht_dir_path, strerror (errno));
+                ret = -1;
+                goto out;
+        }
+
+        size = sys_fgetxattr (ht_dir_fd, HTIME_CURRENT, ht_file_bname,
+                             sizeof (ht_file_bname));
+        if (size < 0) {
+                gf_log (this->name, GF_LOG_ERROR, "Error extracting"
+                        " HTIME_CURRENT: %s.", strerror (errno));
+
+                /* If upgrade scenario, find the latest HTIME.TSTAMP file
+                 * and use the same. If error, create a new HTIME.TSTAMP
+                 * file.
+                 */
+                cnt = find_current_htime (ht_dir_fd, ht_dir_path,
+                                           ht_file_bname);
+                if (cnt <= 0)
+                        return htime_create (this, priv, ts);
+        }
+
+        gf_log (this->name, GF_LOG_INFO, "HTIME_CURRENT: %s", ht_file_bname);
+        (void) snprintf (ht_file_path, PATH_MAX, "%s/%s",
+                         ht_dir_path, ht_file_bname);
+
+        /* Open in append mode as existing htime file is used */
+        flags |= (O_RDWR | O_SYNC | O_APPEND);
+        ht_file_fd = open (ht_file_path, flags,
+                        S_IRUSR | S_IWUSR | S_IRGRP | S_IROTH);
+        if (ht_file_fd < 0) {
+                gf_log (this->name, GF_LOG_ERROR,
+                        "unable to open htime file: %s"
+                        "(reason: %s)", ht_file_path, strerror (errno));
+                ret = -1;
+                goto out;
+        }
+
+        /* save this htime_fd in priv->htime_fd */
+        priv->htime_fd = ht_file_fd;
+
+        /* Initialize rollover-number in priv to current number */
+        size = sys_fgetxattr (ht_file_fd, HTIME_KEY, x_value, sizeof (x_value));
+        if (size < 0) {
+                gf_log (this->name, GF_LOG_ERROR, "error extracting max"
+                        " timstamp from htime file %s (reason %s)",
+                        ht_file_path, strerror (errno));
+                ret = -1;
+                goto out;
+        }
+
+        sscanf (x_value, "%lu:%lu", &max_ts, &total);
+        gf_log (this->name, GF_LOG_INFO, "INIT CASE: MIN: %lu, MAX: %lu,"
+                " TOTAL CHANGELOGS: %lu", min_ts, max_ts, total);
+        priv->rollover_count = total + 1;
+
+out:
+        if (ht_dir_fd != -1)
+                close (ht_dir_fd);
+        return ret;
+}
+
+/* Returns 0 on successful creation of htime file
+ * returns -1 on failure or error
+ */
+int
+htime_create (xlator_t *this,
+              changelog_priv_t *priv, unsigned long ts)
+{
+        int ht_file_fd                  = -1;
+        int ht_dir_fd                   = -1;
+        int ret                         = 0;
+        char ht_dir_path[PATH_MAX]      = {0,};
+        char ht_file_path[PATH_MAX]     = {0,};
+        char ht_file_bname[NAME_MAX]    = {0,};
         int flags                       = 0;
 
         CHANGELOG_FILL_HTIME_DIR(priv->changelog_dir, ht_dir_path);
@@ -442,18 +602,17 @@ htime_open (xlator_t *this,
                         HTIME_FILE_NAME, ts);
 
         flags |= (O_CREAT | O_RDWR | O_SYNC);
-        fd = open (ht_file_path, flags,
+        ht_file_fd = open (ht_file_path, flags,
                         S_IRUSR | S_IWUSR | S_IRGRP | S_IROTH);
-        if (fd < 0) {
+        if (ht_file_fd < 0) {
                 gf_log (this->name, GF_LOG_ERROR,
-                        "unable to open/create htime file: %s"
+                        "unable to create htime file: %s"
                         "(reason: %s)", ht_file_path, strerror (errno));
                 ret = -1;
                 goto out;
-
         }
 
-        if (sys_fsetxattr (fd, HTIME_KEY, HTIME_INITIAL_VALUE,
+        if (sys_fsetxattr (ht_file_fd, HTIME_KEY, HTIME_INITIAL_VALUE,
                        sizeof (HTIME_INITIAL_VALUE)-1,  0)) {
                 gf_log (this->name, GF_LOG_ERROR,
                         "Htime xattr initialization failed");
@@ -461,12 +620,47 @@ htime_open (xlator_t *this,
                 goto out;
         }
 
+        ret = fsync (ht_file_fd);
+        if (ret < 0) {
+                gf_log (this->name, GF_LOG_ERROR, "fsync failed (reason: %s)",
+                        strerror (errno));
+                goto out;
+        }
+
+        /* Set xattr HTIME_CURRENT on htime directory to htime filename */
+        ht_dir_fd = open (ht_dir_path, O_RDONLY);
+        if (ht_dir_fd == -1) {
+                gf_log (this->name, GF_LOG_ERROR, "open of %s failed: %s",
+                        ht_dir_path, strerror (errno));
+                ret = -1;
+                goto out;
+        }
+
+        (void) snprintf (ht_file_bname, PATH_MAX, "%s.%lu",
+                         HTIME_FILE_NAME, ts);
+        if (sys_fsetxattr (ht_dir_fd, HTIME_CURRENT, ht_file_bname,
+            strlen (ht_file_bname), 0)) {
+                gf_log (this->name, GF_LOG_ERROR, "fsetxattr failed:"
+                        " HTIME_CURRENT: %s", strerror (errno));
+                ret = -1;
+                goto out;
+        }
+
+        ret = fsync (ht_dir_fd);
+        if (ret < 0) {
+                gf_log (this->name, GF_LOG_ERROR, "fsync failed (reason: %s)",
+                        strerror (errno));
+                goto out;
+        }
+
         /* save this htime_fd in priv->htime_fd */
-        priv->htime_fd = fd;
+        priv->htime_fd = ht_file_fd;
         /* initialize rollover-number in priv to 1 */
         priv->rollover_count = 1;
 
 out:
+        if (ht_dir_fd != -1)
+                close (ht_dir_fd);
         return ret;
 }
 
diff --git a/xlators/features/changelog/src/changelog-helpers.h b/xlators/features/changelog/src/changelog-helpers.h
index 765d5fd878b..b552c308e0f 100644
--- a/xlators/features/changelog/src/changelog-helpers.h
+++ b/xlators/features/changelog/src/changelog-helpers.h
@@ -432,7 +432,9 @@ int
 htime_update (xlator_t *this, changelog_priv_t *priv,
               unsigned long ts, char * buffer);
 int
-htime_open (xlator_t *this, changelog_priv_t * priv, unsigned long ts);
+htime_open (xlator_t *this, changelog_priv_t *priv, unsigned long ts);
+int
+htime_create (xlator_t *this, changelog_priv_t *priv, unsigned long ts);
 
 /* Geo-Rep snapshot dependency changes */
 void
diff --git a/xlators/features/changelog/src/changelog-misc.h b/xlators/features/changelog/src/changelog-misc.h
index 196bd4c8195..0de0edd9516 100644
--- a/xlators/features/changelog/src/changelog-misc.h
+++ b/xlators/features/changelog/src/changelog-misc.h
@@ -19,6 +19,7 @@
 #define HTIME_FILE_NAME "HTIME"
 #define CSNAP_FILE_NAME "CHANGELOG.SNAP"
 #define HTIME_KEY "trusted.glusterfs.htime"
+#define HTIME_CURRENT "trusted.glusterfs.current_htime"
 #define HTIME_INITIAL_VALUE "0:0"
 
 #define CHANGELOG_VERSION_MAJOR  1
diff --git a/xlators/features/changelog/src/changelog.c b/xlators/features/changelog/src/changelog.c
index df8172765d7..7429fc5e891 100644
--- a/xlators/features/changelog/src/changelog.c
+++ b/xlators/features/changelog/src/changelog.c
@@ -2359,13 +2359,16 @@ reconfigure (xlator_t *this, dict_t *options)
 
                 if (active_now) {
                         if (!active_earlier) {
+                                gf_log (this->name, GF_LOG_INFO,
+                                        "Changelog enable: Creating new "
+                                        "HTIME.TSTAMP file");
                                 if (gettimeofday(&tv, NULL) ) {
                                         gf_log (this->name, GF_LOG_ERROR,
                                                  "unable to fetch htime");
                                         ret = -1;
                                         goto out;
                                 }
-                                htime_open(this, priv, tv.tv_sec);
+                                htime_create (this, priv, tv.tv_sec);
                         }
                         ret = changelog_spawn_helper_threads (this, priv);
                 }