Skip to content
GitLab
Projects Groups Topics Snippets
  • /
  • Help
    • Help
    • Support
    • Community forum
    • Submit feedback
    • Contribute to GitLab
  • Register
  • Sign in
  • Kea Kea
  • Project information
    • Project information
    • Activity
    • Labels
    • Members
  • Repository
    • Repository
    • Files
    • Commits
    • Branches
    • Tags
    • Contributor statistics
    • Graph
    • Compare revisions
  • Issues 595
    • Issues 595
    • List
    • Boards
    • Service Desk
    • Milestones
  • Merge requests 62
    • Merge requests 62
  • Deployments
    • Deployments
    • Releases
  • Packages and registries
    • Packages and registries
    • Container Registry
    • Model experiments
  • Monitor
    • Monitor
    • Incidents
  • Analytics
    • Analytics
    • Value stream
    • Repository
  • Wiki
    • Wiki
  • Snippets
    • Snippets
  • Activity
  • Graph
  • Create a new issue
  • Commits
  • Issue Boards
Collapse sidebar
  • ISC Open Source ProjectsISC Open Source Projects
  • KeaKea
  • Issues
  • #2507
Closed
Open
Issue created Jul 25, 2022 by Thomas Markwalder@tmarkMaintainer

Replace use of CriticalSection in stat-leaseX-get commands with Memfile_LeaseMgr mutex lock

The command handlers for stat-leaseX-get commands create CriticalSections upon entry. We should be able to replace those with simply locking the Memfile lease manager's mutex like so:

tmark@wild-dog:stat_cmds (master) $ git diff
diff --git a/src/hooks/dhcp/stat_cmds/stat_cmds.cc b/src/hooks/dhcp/stat_cmds/stat_cmds.cc
index 8950bc928f..b9fe1800d2 100644
--- a/src/hooks/dhcp/stat_cmds/stat_cmds.cc
+++ b/src/hooks/dhcp/stat_cmds/stat_cmds.cc
@@ -718,7 +718,9 @@ int
 StatCmds::statLease4GetHandler(CalloutHandle& handle) {
     try {
         LeaseStatCmdsImpl impl;
+#if 0
         MultiThreadingCriticalSection cs;
+#endif
         return (impl.statLease4GetHandler(handle));
     } catch (const std::exception& ex) {
 
diff --git a/src/lib/dhcpsrv/memfile_lease_mgr.cc b/src/lib/dhcpsrv/memfile_lease_mgr.cc
index afb3259840..86f95d2cd9 100644
--- a/src/lib/dhcpsrv/memfile_lease_mgr.cc
+++ b/src/lib/dhcpsrv/memfile_lease_mgr.cc
@@ -1990,7 +1990,16 @@ Memfile_LeaseMgr::lfcExecute(boost::shared_ptr<LeaseFileType>& lease_file) {
 LeaseStatsQueryPtr
 Memfile_LeaseMgr::startLeaseStatsQuery4() {
     LeaseStatsQueryPtr query(new MemfileLeaseStatsQuery4(storage4_));
+#if 1
+    if (MultiThreadingMgr::instance().getMode()) {
+        std::lock_guard<std::mutex> lock(*mutex_);
+        query->start();
+    } else {
+        query->start();
+    }
+#else
     query->start();
+#endif
     return(query);
 }

I compiled with thread-sanitizer on Ubuntu 20.04, and ran kea-dhcp4 stand-alone in MT mode (8 threads), used traffic. I used a script to call socat locally to generate stat-lease4-get() at a rate every 100 ms. Each test started with an empty lease file and ran for sixty seconds. I tested three variations of the code 1: with CriticalSection 2: without CriticalSection or Mutex 3: With Mutex. The results are attached:

out.txt

All three versions report one or two race warnings in StringSanitizerImpl, this is known false report. The version without CS or mutex had almost 90 race warnings emanating from alloc engine, which one would expect. The mutex-ed version is slightly faster than the CriticalSection version, and I suspect the difference would be larger if running HA+MT as a CS would cause both thread pools to pause/resume.

The stat commands do not need to create a CriticalSection and therefore should not. We might consider the same approach for the lease wipe commands.

To upload designs, you'll need to enable LFS and have an admin enable hashed storage. More information
Assignee
Assign to
Time tracking