Merge pull request #21 from adam-p/no-store-time-diff

don't immediately write server time diff to datastore

Author: adam-p

datastore.cpp

@@ -131,13 +131,15 @@ error::Result<nlohmann::json> Datastore::Get() const {
     return json_;
 }
 
-Error Datastore::Set(const json::json_pointer& p, json v) {
+Error Datastore::Set(const json::json_pointer& p, json v, bool write_store/*=true*/) {
     SYNCHRONIZE(mutex_);
     MUST_BE_INITIALIZED;
 
     // We will use the transaction mechanism to do the writing. It will also help prevent
     // changes to the stored value between the time we check it and the time we set it.
-    BeginTransaction();
+    if (write_store) {
+        BeginTransaction();
+    }
 
     // Avoid modifying the datastore if the value is the same as what's already there.
     bool changed = true;
@@ -151,7 +153,10 @@ Error Datastore::Set(const json::json_pointer& p, json v) {
     json_[p] = v;
     transaction_dirty_ = changed;
 
-    return PassError(EndTransaction(true));
+    if (write_store) {
+        return PassError(EndTransaction(true));
+    }
+    return nullerr;
 }
 
 static string FilePath(const string& file_root, const string& suffix) {

datastore.hpp

@@ -105,11 +105,13 @@ class Datastore {
 
     error::Result<nlohmann::json> Get() const;
 
-    // Sets the value v in the datastore at path p.
+    /// Sets the value v in the datastore at path p.
+    /// If write_store is false, the change will stored in memory but not written to disk.
+    /// However, the dirty flag will be set and the change will be in the next write.
     /// NOTE: Set is not atomic. If the file operation fails, the intermediate object will still be
     /// updated. We may want this to be otherwise in the future, but for now I think that it's preferable.
     /// Returns false if the file operation failed.
-    error::Error Set(const json::json_pointer& p, json v);
+    error::Error Set(const json::json_pointer& p, json v, bool write_store=true);
 
 protected:
     /// Helper for the public Reset methods

userdata.cpp

@@ -196,7 +196,11 @@ datetime::Duration UserData::GetServerTimeDiff() const {
 error::Error UserData::SetServerTimeDiff(const datetime::DateTime& serverTimeNow) {
     auto localTimeNow = datetime::DateTime::Now();
     auto diff = serverTimeNow.Diff(localTimeNow);
-    return PassError(datastore_.Set(kServerTimeDiffPtr, datetime::DurationToInt64(diff)));
+    // Updating the server time diff isn't so important that it needs to be written to disk
+    // immediately. Also, it is generally done outside of a transaction and then followed
+    // by a transaction, so it can lead to rapid datastore updates (which we suspect can
+    // cause corruption issues).
+    return PassError(datastore_.Set(kServerTimeDiffPtr, datetime::DurationToInt64(diff), /*write_store=*/false));
 }
 
 datetime::DateTime UserData::ServerTimeToLocal(const datetime::DateTime& server_time) const {