527848 - make sure db upgrade to 4.7 and later works correctly

https://bugzilla.redhat.com/show_bug.cgi?id=527848

Change Description:
1. Replication Changelog
1-1. In the clean recover mode, transaction logs should not be removed.
1-2. When nsslapd-db-circular-logging is on (by default, it's on),
     call log_archive function with DB_ARCH_REMOVE, which removes
     log files that are no longer needed.
1-3. Call transaction checkpoint just before shutting down the server.
1-4. "From string" in the upbrade message had a flaw.
2. Backend dblayer
2-1. In checkpoint_threadmain, call log_archive with DB_ARCH_ABS,
     which returns the absolute path of the transaction log files.
	 It eliminates the code which generates the absolute paths.

ldap/servers/plugins/replication/cl5_api.c

@@ -2396,7 +2396,6 @@ static int _cl5AppInit (PRBool *didRecovery)
 		if (CL5_OPEN_CLEAN_RECOVER == s_cl5Desc.dbOpenMode)
 		{
 			_cl5RemoveEnv();
-			_cl5RemoveLogs();
 		}
 
 		rc = _cl5Recover (flags, dbEnv);
@@ -3267,7 +3266,7 @@ static int  _cl5CheckpointMain (void *param)
 			}
 #if 1000*DB_VERSION_MAJOR + 100*DB_VERSION_MINOR < 4100
 			else if (rc != DB_INCOMPLETE) /* real error happened */
-			{			
+			{
 				slapi_log_error(SLAPI_LOG_FATAL, repl_plugin_name_cl, 
 					"_cl5CheckpointMain: checkpoint failed, db error - %d %s\n",
 					rc, db_strerror(rc));
@@ -3278,22 +3277,25 @@ static int  _cl5CheckpointMain (void *param)
 			if (s_cl5Desc.dbConfig.circularLogging)
 			{
 				char **list = NULL;
-				char **listp = NULL;
-				int rc  = -1;
-				char filename[MAXPATHLEN + 1];
-
 				/* find out which log files don't contain active txns */
-				rc = LOG_ARCHIVE(s_cl5Desc.dbEnv, &list, 0, (void *)slapi_ch_malloc);
-				if (0 == rc && NULL != list)
+				/* DB_ARCH_REMOVE: Remove log files that are no longer needed; 
+				 * no filenames are returned. */
+				int rc  = LOG_ARCHIVE(s_cl5Desc.dbEnv, &list,
+									  DB_ARCH_REMOVE, (void *)slapi_ch_malloc);
+				if (rc)
 				{
-					/* zap 'em ! */
-					for (listp = list; *listp != NULL; ++listp)
-					{	
-						PR_snprintf(filename, MAXPATHLEN, "%s/%s", s_cl5Desc.dbDir,*listp);
-                        PR_Delete (filename);
-					}
-					slapi_ch_free((void **)&list);
+					slapi_log_error(SLAPI_LOG_FATAL, repl_plugin_name_cl, 
+									"_cl5CheckpointMain: log archive failed, "
+									"db error - %d %s\n", rc, db_strerror(rc));
 				}
+				slapi_ch_free((void **)&list); /* just in case */
+			}
+			else
+			{
+				slapi_log_error(SLAPI_LOG_REPL, repl_plugin_name_cl,
+								"_cl5CheckpointMain: %s is off; "
+								"transaction logs won't be removed.\n", 
+								CONFIG_CHANGELOG_DB_CIRCULAR_LOGGING);
 			}
 		}
 
@@ -3302,6 +3304,8 @@ static int  _cl5CheckpointMain (void *param)
 		/* answer---because the interval might be changed after the server starts up */
 		DS_Sleep(interval);
 	}
+	/* Check point and archive before shutting down */
+	rc = TXN_CHECKPOINT(s_cl5Desc.dbEnv, 0, 0, 0);
 
 	PR_AtomicDecrement (&s_cl5Desc.threadCount);
 	slapi_log_error(SLAPI_LOG_REPL, repl_plugin_name_cl, "_cl5CheckpointMain: exiting\n");
@@ -3536,7 +3540,7 @@ static int _cl5CheckDBVersion ()
 		{
 			*dotp = '\0';
 			dbmajor = strtol(versionp, (char **)NULL, 10);
-			dbminor = strtol(++dotp, (char **)NULL, 10);
+			dbminor = strtol(dotp+1, (char **)NULL, 10);
 			*dotp = '.';
 		}
 		else
@@ -3991,6 +3995,8 @@ static int  _cl5Delete (const char *clDir, int rmDir)
 			continue;
 		}
 		PR_snprintf(filename, MAXPATHLEN, "%s/%s", clDir, entry->name);
+		/* _cl5Delete deletes the whole changelog directory with all the files 
+		 * underneath.  Thus, we can just remove them physically. */
 		rc = PR_Delete(filename);
 		if (rc != PR_SUCCESS)
 		{

ldap/servers/slapd/back-ldbm/dblayer.c

@@ -2485,10 +2485,11 @@ int dblayer_instance_close(backend *be)
     }
     inst->inst_id2entry = NULL;
 
-     if (inst->import_env) {
-       /* ignore the value of env, close, because at this point, work is done with import env
-      by calling env.close, env and all the associated db handles will be closed, ignore,
-          if sleepycat complains, that db handles are open at env close time */
+    if (inst->import_env) {
+       /* ignore the value of env, close, because at this point, 
+        * work is done with import env by calling env.close, 
+        * env and all the associated db handles will be closed, ignore,
+        * if sleepycat complains, that db handles are open at env close time */
        return_value |= inst->import_env->dblayer_DB_ENV->close(inst->import_env->dblayer_DB_ENV, 0);
        return_value = db_env_create(&env, 0);
        if (return_value == 0) {
@@ -2510,9 +2511,9 @@ int dblayer_instance_close(backend *be)
        }
        PR_DestroyRWLock(inst->import_env->dblayer_env_lock);
        slapi_ch_free((void **)&inst->import_env);
-     } else {
+    } else {
        be->be_state = BE_STATE_STOPPED;
-     }
+    }
 
     return return_value;
 }
@@ -3496,7 +3497,7 @@ dblayer_start_deadlock_thread(struct ldbminfo *li)
     return return_value;
 }
 
-/* checkpoint thread main function */
+/* deadlock thread main function */
 
 static int deadlock_threadmain(void *param)
 {
@@ -3643,6 +3644,9 @@ static int checkpoint_threadmain(void *param)
     int debug_checkpointing = 0;
     int checkpoint_interval;
     char *home_dir = NULL;
+    char **list = NULL;
+    char **listp = NULL;
+    struct dblayer_private_env *penv = NULL;
 
     PR_ASSERT(NULL != param);
     li = (struct ldbminfo*)param;
@@ -3665,6 +3669,7 @@ static int checkpoint_threadmain(void *param)
     /* work around a problem with newly created environments */
     dblayer_force_checkpoint(li);
 
+    penv = priv->dblayer_env;
     debug_checkpointing = priv->db_debug_checkpointing;
     /* assumes dblayer_force_checkpoint worked */
     time_of_last_checkpoint_completion = current_time();
@@ -3745,59 +3750,41 @@ static int checkpoint_threadmain(void *param)
                 time_of_last_checkpoint_completion = current_time();
             }
         }
-        {
-            char **list = NULL;
-            char **listp = NULL;
-            int return_value  = -1;
-            char filename[MAXPATHLEN];
-            char *prefix = NULL;
-            struct dblayer_private_env *penv = priv->dblayer_env;
-            if ((NULL != priv->dblayer_log_directory) &&
-                (0 != strlen(priv->dblayer_log_directory))) 
-            {
-                prefix = priv->dblayer_log_directory;
-            } 
-            else 
-            {
-                prefix = home_dir;
-            }
-            /* find out which log files don't contain active txns */
-            DB_CHECKPOINT_LOCK(PR_TRUE, penv->dblayer_env_lock);
-            return_value = LOG_ARCHIVE(penv->dblayer_DB_ENV, &list,
-                                       0, (void *)slapi_ch_malloc);
-            DB_CHECKPOINT_UNLOCK(PR_TRUE, penv->dblayer_env_lock);
-            checkpoint_debug_message(debug_checkpointing,
-                "Got list of logfiles not needed %d %p\n",
-                return_value,list, 0);
-            if (0 == return_value && NULL != list)
-            {
-                /* zap 'em ! */
-                for (listp = list; *listp != NULL; ++listp)
-                {    
-                    PR_snprintf(filename,sizeof(filename),"%s/%s",prefix,*listp);
-                    if (priv->dblayer_circular_logging) {
-                        checkpoint_debug_message(debug_checkpointing,
-                            "Deleting %s\n",filename, 0, 0);
-                        unlink(filename);    
-                    } else {
-                        char new_filename[MAXPATHLEN];
-                        PR_snprintf(new_filename,sizeof(new_filename),"%s/old.%s",
-                                prefix,*listp);
-                        checkpoint_debug_message(debug_checkpointing,
-                            "Renaming %s\n",filename,0, 0);
-                        rename(filename,new_filename);    
-                    }
+        /* find out which log files don't contain active txns */
+        DB_CHECKPOINT_LOCK(PR_TRUE, penv->dblayer_env_lock);
+        rval = LOG_ARCHIVE(penv->dblayer_DB_ENV, &list,
+                           DB_ARCH_ABS, (void *)slapi_ch_malloc);
+        DB_CHECKPOINT_UNLOCK(PR_TRUE, penv->dblayer_env_lock);
+        if (rval) {
+            LDAPDebug2Args(LDAP_DEBUG_ANY, "checkpoint_threadmain: "
+                           "log archive failed - %s (%d)\n", 
+                           dblayer_strerror(rval), rval);
+        } else {
+            for (listp = list; listp && *listp != NULL; ++listp) {
+                if (priv->dblayer_circular_logging) {
+                    checkpoint_debug_message(debug_checkpointing,
+                                             "Deleting %s\n", *listp, 0, 0);
+                    unlink(*listp);
+                } else {
+                    char new_filename[MAXPATHLEN];
+                    PR_snprintf(new_filename, sizeof(new_filename),
+                                "%s.old", *listp);
+                    checkpoint_debug_message(debug_checkpointing,
+                                "Renaming %s -> %s\n",*listp, new_filename, 0);
+                    rename(*listp, new_filename);    
                 }
-                slapi_ch_free((void**)&list);
             }
+            slapi_ch_free((void**)&list);
+            /* Note: references inside the returned memory need not be 
+             * individually freed. */
         }
     }
-    LDAPDebug(LDAP_DEBUG_TRACE, "Leaving checkpoint_threadmain before checkpoint\n", 0, 0, 0);
+    LDAPDebug0Args(LDAP_DEBUG_TRACE, "Check point before leaving\n");
     rval = dblayer_force_checkpoint(li);
 error_return:
 
     DECR_THREAD_COUNT(priv);
-    LDAPDebug(LDAP_DEBUG_TRACE, "Leaving checkpoint_threadmain\n", 0, 0, 0);
+    LDAPDebug0Args(LDAP_DEBUG_TRACE, "Leaving checkpoint_threadmain\n");
     return rval;
 }