Network batching

src/sm/database/database.cpp

@@ -812,6 +812,57 @@
     return ErrorEnum::eNone;
 }
 
+Error Database::BeginTransaction()
+{
+    std::lock_guard lock {mMutex};
+
+    LOG_DBG() << "Begin transaction";
+
+    try {
+        if (!mSession->isTransaction()) {
+            mSession->begin();
+        }
+    } catch (const std::exception& e) {
+        return AOS_ERROR_WRAP(common::utils::ToAosError(e));
+    }
+
+    return ErrorEnum::eNone;
+}
+
+Error Database::CommitTransaction()
+{
+    std::lock_guard lock {mMutex};
+
+    LOG_DBG() << "Commit transaction";
+
+    try {
+        if (mSession->isTransaction()) {
+            mSession->commit();
+        }
+    } catch (const std::exception& e) {
+        return AOS_ERROR_WRAP(common::utils::ToAosError(e));
+    }
+
+    return ErrorEnum::eNone;
+}
+
+Error Database::RollbackTransaction()
+{
+    std::lock_guard lock {mMutex};
+
+    LOG_DBG() << "Rollback transaction";
+
+    try {
+        if (mSession->isTransaction()) {
+            mSession->rollback();
+        }
+    } catch (const std::exception& e) {
+        return AOS_ERROR_WRAP(common::utils::ToAosError(e));
+    }
+
+    return ErrorEnum::eNone;
+}
+
 Error Database::AddInstanceNetworkInfo(const sm::networkmanager::InstanceNetworkInfo& info)
 {
     std::lock_guard lock {mMutex};

src/sm/database/database.hpp

@@ -23,7 +23,7 @@
 
 namespace aos::sm::database {
 
 class Database : public DatabaseItf, public sm::alerts::StorageItf {
 public:
     /**
      * Creates database instance.
@@ -208,6 +208,28 @@
      */
     Error RemoveTrafficMonitorData(const String& chain) override;
 
+    /**
+     * Begins a SQLite transaction so subsequent writes commit together (one
+     * fsync) on CommitTransaction.
+     *
+     * @return Error.
+     */
+    Error BeginTransaction() override;
+
+    /**
+     * Commits the current SQLite transaction.
+     *
+     * @return Error.
+     */
+    Error CommitTransaction() override;
+
+    /**
+     * Rolls back the current SQLite transaction, discarding its writes.
+     *
+     * @return Error.
+     */
+    Error RollbackTransaction() override;
+
     // sm::alerts::StorageItf interface
 
     /**

src/sm/database/tests/database.cpp

@@ -622,6 +622,102 @@ TEST_F(DatabaseTest, SetUpdateAndRemoveTrafficMonitorDataSucceeds)
     ASSERT_TRUE(mDB.GetTrafficMonitorData(chain, resTime, resValue).Is(aos::ErrorEnum::eNotFound));
 }
 
+TEST_F(DatabaseTest, TransactionCommitPersistsWrites)
+{
+    ASSERT_TRUE(mDB.Init(mWorkingDir.string(), mMigrationConfig).IsNone());
+
+    aos::sm::networkmanager::InstanceNetworkInfo info1;
+    info1.mInstanceID = "instance-1";
+    info1.mNetworkID  = "network-1";
+    info1.mHostIfName = "veth-initial";
+
+    aos::sm::networkmanager::InstanceNetworkInfo info2;
+    info2.mInstanceID = "instance-2";
+    info2.mNetworkID  = "network-2";
+
+    ASSERT_TRUE(mDB.BeginTransaction().IsNone());
+
+    ASSERT_TRUE(mDB.AddInstanceNetworkInfo(info1).IsNone());
+
+    info1.mHostIfName = "veth-updated";
+
+    ASSERT_TRUE(mDB.UpdateInstanceNetworkInfo(info1).IsNone());
+    ASSERT_TRUE(mDB.AddInstanceNetworkInfo(info2).IsNone());
+
+    ASSERT_TRUE(mDB.CommitTransaction().IsNone());
+
+    aos::StaticArray<aos::sm::networkmanager::InstanceNetworkInfo, 2> result;
+
+    ASSERT_TRUE(mDB.GetInstanceNetworksInfo(result).IsNone());
+
+    ASSERT_EQ(result.Size(), 2);
+
+    EXPECT_EQ(result[0].mInstanceID, info1.mInstanceID);
+    EXPECT_EQ(result[0].mHostIfName, info1.mHostIfName);
+    EXPECT_EQ(result[1].mInstanceID, info2.mInstanceID);
+}
+
+TEST_F(DatabaseTest, TransactionRollbackDiscardsWrites)
+{
+    ASSERT_TRUE(mDB.Init(mWorkingDir.string(), mMigrationConfig).IsNone());
+
+    aos::sm::networkmanager::InstanceNetworkInfo committed;
+    committed.mInstanceID = "instance-0";
+    committed.mNetworkID  = "network-0";
+
+    ASSERT_TRUE(mDB.AddInstanceNetworkInfo(committed).IsNone());
+
+    aos::sm::networkmanager::InstanceNetworkInfo staged;
+    staged.mInstanceID = "instance-1";
+    staged.mNetworkID  = "network-1";
+
+    ASSERT_TRUE(mDB.BeginTransaction().IsNone());
+
+    ASSERT_TRUE(mDB.AddInstanceNetworkInfo(staged).IsNone());
+    ASSERT_TRUE(mDB.SetTrafficMonitorData("chain", aos::Time::Now(), 100).IsNone());
+
+    ASSERT_TRUE(mDB.RollbackTransaction().IsNone());
+
+    aos::StaticArray<aos::sm::networkmanager::InstanceNetworkInfo, 2> result;
+
+    ASSERT_TRUE(mDB.GetInstanceNetworksInfo(result).IsNone());
+
+    ASSERT_EQ(result.Size(), 1);
+    EXPECT_EQ(result[0].mInstanceID, committed.mInstanceID);
+
+    aos::Time resTime;
+    uint64_t  resValue = 0;
+
+    EXPECT_TRUE(mDB.GetTrafficMonitorData("chain", resTime, resValue).Is(aos::ErrorEnum::eNotFound));
+}
+
+TEST_F(DatabaseTest, TransactionCommitAndRollbackWithoutBeginAreNoOp)
+{
+    ASSERT_TRUE(mDB.Init(mWorkingDir.string(), mMigrationConfig).IsNone());
+
+    EXPECT_TRUE(mDB.CommitTransaction().IsNone());
+    EXPECT_TRUE(mDB.RollbackTransaction().IsNone());
+
+    aos::sm::networkmanager::InstanceNetworkInfo info;
+    info.mInstanceID = "instance-1";
+    info.mNetworkID  = "network-1";
+
+    ASSERT_TRUE(mDB.AddInstanceNetworkInfo(info).IsNone());
+
+    ASSERT_TRUE(mDB.BeginTransaction().IsNone());
+    ASSERT_TRUE(mDB.CommitTransaction().IsNone());
+
+    EXPECT_TRUE(mDB.CommitTransaction().IsNone());
+    EXPECT_TRUE(mDB.RollbackTransaction().IsNone());
+
+    aos::StaticArray<aos::sm::networkmanager::InstanceNetworkInfo, 1> result;
+
+    ASSERT_TRUE(mDB.GetInstanceNetworksInfo(result).IsNone());
+
+    ASSERT_EQ(result.Size(), 1);
+    EXPECT_EQ(result[0].mInstanceID, info.mInstanceID);
+}
+
 /***********************************************************************************************************************
  * Tests - alerts::StorageItf
  **********************************************************************************************************************/

src/sm/networkmanager/bandwidth.cpp

@@ -122,31 +122,13 @@ Error Bandwidth::Clear(const String& ifName)
 {
     LOG_DBG() << "Clear bandwidth" << Log::Field("ifName", ifName);
 
-    Error err;
-
-    if (auto rootErr = mTC->DelRootTBFQDisc(ifName); !rootErr.IsNone()) {
-        LOG_ERR() << "Failed to delete root TBF qdisc" << Log::Field(rootErr);
-
-        err = AOS_ERROR_WRAP(rootErr);
-    }
-
-    if (auto ingErr = mTC->DelIngressQDisc(ifName); !ingErr.IsNone()) {
-        LOG_ERR() << "Failed to delete ingress qdisc" << Log::Field(ingErr);
-
-        if (err.IsNone()) {
-            err = AOS_ERROR_WRAP(ingErr);
-        }
-    }
-
-    if (auto ifbErr = mIfMgr->DeleteLink(IFBName(ifName)); !ifbErr.IsNone() && !ifbErr.Is(ErrorEnum::eNotFound)) {
-        LOG_ERR() << "Failed to delete IFB" << Log::Field(ifbErr);
+    if (auto err = mIfMgr->DeleteLink(IFBName(ifName)); !err.IsNone() && !err.Is(ErrorEnum::eNotFound)) {
+        LOG_ERR() << "Failed to delete IFB" << Log::Field(err);
 
-        if (err.IsNone()) {
-            err = AOS_ERROR_WRAP(ifbErr);
-        }
+        return AOS_ERROR_WRAP(err);
     }
 
-    return err;
+    return ErrorEnum::eNone;
 }
 
 /***********************************************************************************************************************