From 64faf87f4041d2f253742f3503c34c3066521e6c Mon Sep 17 00:00:00 2001
From: Roman Gershman <roman@dragonflydb.io>
Date: Sun, 8 Sep 2024 09:24:09 +0300
Subject: [PATCH] fix: recursive calls in the allocation tracker (#3665)

Also, remove dependence of absl::TimeZone bloated monstrosity, which was required by
absl::FormatTime api, even though we do not actually format a timezone.

When absl::LocalTimeZone is accessed it allocates hundreds of thousands of bytes
for each shard thread (maybe due to lack thread safety during lazy initialization).

At the end, strftime does a great job without any shenanigans.

Signed-off-by: Roman Gershman <roman@dragonflydb.io>
---
 src/core/allocation_tracker.cc              | 13 +++++++++----
 src/core/allocation_tracker.h               |  1 +
 src/server/detail/save_stages_controller.cc | 19 ++++++++++++++-----
 src/server/detail/save_stages_controller.h  |  2 +-
 4 files changed, 25 insertions(+), 10 deletions(-)

diff --git a/src/core/allocation_tracker.cc b/src/core/allocation_tracker.cc
index d0fc5235cc06..7b65c3b22c51 100644
--- a/src/core/allocation_tracker.cc
+++ b/src/core/allocation_tracker.cc
@@ -53,13 +53,12 @@ void AllocationTracker::ProcessNew(void* ptr, size_t size) {
     return;
   }
 
-  thread_local bool inside_process_new = false;
-  if (inside_process_new) {
+  if (inside_tracker_) {
     return;
   }
 
   // Prevent endless recursion, in case logging allocates memory
-  inside_process_new = true;
+  inside_tracker_ = true;
   double random = absl::Uniform(g_bitgen, 0.0, 1.0);
   for (const auto& band : tracking_) {
     if (random >= band.sample_odds || size > band.upper_bound || size < band.lower_bound) {
@@ -72,10 +71,15 @@ void AllocationTracker::ProcessNew(void* ptr, size_t size) {
               << "). Stack: " << util::fb2::GetStacktrace();
     break;
   }
-  inside_process_new = false;
+  inside_tracker_ = false;
 }
 
 void AllocationTracker::ProcessDelete(void* ptr) {
+  if (inside_tracker_) {
+    return;
+  }
+
+  inside_tracker_ = true;
   // we partially handle deletes, specifically when specifying a single range with
   // 100% sampling rate.
   if (tracking_.size() == 1 && tracking_.front().sample_odds == 1) {
@@ -84,6 +88,7 @@ void AllocationTracker::ProcessDelete(void* ptr) {
       LOG(INFO) << "Deallocating " << usable << " bytes (" << ptr << ")";
     }
   }
+  inside_tracker_ = false;
 }
 
 }  // namespace dfly
diff --git a/src/core/allocation_tracker.h b/src/core/allocation_tracker.h
index 21c4b7f8c9a4..58d3bd057c42 100644
--- a/src/core/allocation_tracker.h
+++ b/src/core/allocation_tracker.h
@@ -47,6 +47,7 @@ class AllocationTracker {
 
  private:
   absl::InlinedVector<TrackingInfo, 4> tracking_;
+  bool inside_tracker_ = false;
 };
 
 }  // namespace dfly
diff --git a/src/server/detail/save_stages_controller.cc b/src/server/detail/save_stages_controller.cc
index 0a56ca02ebca..5280348b186d 100644
--- a/src/server/detail/save_stages_controller.cc
+++ b/src/server/detail/save_stages_controller.cc
@@ -153,7 +153,7 @@ void RdbSnapshot::StartInShard(EngineShard* shard) {
 
 SaveStagesController::SaveStagesController(SaveStagesInputs&& inputs)
     : SaveStagesInputs{std::move(inputs)} {
-  start_time_ = absl::Now();
+  start_time_ = time(NULL);
 }
 
 SaveStagesController::~SaveStagesController() {
@@ -302,12 +302,12 @@ void SaveStagesController::SaveRdb() {
 }
 
 uint32_t SaveStagesController::GetCurrentSaveDuration() {
-  return (absl::Now() - start_time_) / absl::Seconds(1);
+  return time(nullptr) - start_time_;
 }
 
 SaveInfo SaveStagesController::GetSaveInfo() {
   SaveInfo info;
-  info.save_time = absl::ToUnixSeconds(start_time_);
+  info.save_time = start_time_;
   info.duration_sec = GetCurrentSaveDuration();
 
   if (shared_err_) {
@@ -378,8 +378,17 @@ GenericError SaveStagesController::BuildFullPath() {
 
   SubstituteFilenamePlaceholders(
       &filename, {.ts = "%Y-%m-%dT%H:%M:%S", .year = "%Y", .month = "%m", .day = "%d"});
-  filename = absl::FormatTime(filename.string(), start_time_, absl::LocalTimeZone());
-  full_path_ = dir_path / filename;
+
+  tm time_tm;
+  localtime_r(&start_time_, &time_tm);
+  string src_format = filename.string();
+  string dest_buf(src_format.size() + 128, '\0');
+  size_t len = strftime(dest_buf.data(), dest_buf.size(), src_format.c_str(), &time_tm);
+  if (len == 0)
+    return {"invalid dbfilename format"};
+  dest_buf.resize(len);
+
+  full_path_ = dir_path / dest_buf;
   is_cloud_ = IsCloudPath(full_path_.string());
   return {};
 }
diff --git a/src/server/detail/save_stages_controller.h b/src/server/detail/save_stages_controller.h
index 56a877c35188..646fe25093dc 100644
--- a/src/server/detail/save_stages_controller.h
+++ b/src/server/detail/save_stages_controller.h
@@ -122,7 +122,7 @@ struct SaveStagesController : public SaveStagesInputs {
   void RunStage(void (SaveStagesController::*cb)(unsigned));
 
  private:
-  absl::Time start_time_;
+  time_t start_time_;
   std::filesystem::path full_path_;
   bool is_cloud_;