Merge pull request #114 from Logflare/fix/single-inflight-flush

fix: single in-flight flush call
Logflare · Oct 20, 2024 · 336923a · 336923a
2 parents 2e7b5cc + 7a7af62
commit 336923a
Show file tree

Hide file tree

Showing 3 changed files with 35 additions and 21 deletions.
diff --git a/lib/logflare_logger/batch_cache.ex b/lib/logflare_logger/batch_cache.ex
@@ -27,12 +27,8 @@ defmodule LogflareLogger.BatchCache do
         |> Enum.each(&Repo.delete/1)
       end
 
-      events = pending_events |> Enum.map(& &1.body)
-      events_count = Enum.count(events)
-
-      if events_count >= config.batch_max_size do
-        flush(config)
-      end
+      if pending_events_count >= config.batch_max_size,
+        do: flush(config)
 
       {:ok, :insert_successful}
     else
@@ -45,7 +41,7 @@ defmodule LogflareLogger.BatchCache do
 
     pending_events = pending_events_not_in_flight()
 
-    if not Enum.empty?(pending_events) do
+    if !Enum.empty?(pending_events) && events_in_flight() == [] do
       ples =
         pending_events
         |> Enum.map(fn ple ->

diff --git a/mix.exs b/mix.exs
@@ -4,7 +4,7 @@ defmodule LogflareLogger.MixProject do
   def project do
     [
       app: :logflare_logger_backend,
-      version: "0.11.4",
+      version: "0.11.5-rc.0",
       elixir: "~> 1.8",
       elixirc_paths: elixirc_paths(Mix.env()),
       start_permanent: Mix.env() == :prod,

diff --git a/test/http_backend_gen_event_test.exs b/test/http_backend_gen_event_test.exs
@@ -1,6 +1,6 @@
 defmodule LogflareLogger.HttpBackendTest do
   use ExUnit.Case
-  alias LogflareLogger.{HttpBackend, Formatter, BatchCache}
+  alias LogflareLogger.{HttpBackend, Formatter, BatchCache, Repo, PendingLoggerEvent}
   use Placebo
 
   @default_config [
@@ -14,7 +14,7 @@ defmodule LogflareLogger.HttpBackendTest do
     metadata: []
   ]
 
-  setup_all do
+  setup do
     on_exit(fn ->
       BatchCache.clear()
       Logger.flush()
@@ -50,28 +50,39 @@ defmodule LogflareLogger.HttpBackendTest do
 
       {:ok, state} = init_with_default(flush_interval: 60_000)
 
-      Enum.reduce(
-        1..10,
-        state,
-        fn i, acc ->
-          msg = {:info, nil, {Logger, "log message", ts(i), []}}
-          {:ok, state} = HttpBackend.handle_event(msg, acc)
-          state
-        end
-      )
+      generate_logs(state, @default_config[:batch_max_size])
 
       Process.sleep(200)
 
       assert_called(
         LogflareApiClient.post_logs(
           any(),
           is(fn batch ->
-            assert length(batch) == 10
+            assert length(batch) == @default_config[:batch_max_size]
           end),
           any()
-        )
+        ),
+        once()
       )
     end
+
+    test "flush not called if log events are in flight" do
+      allow(LogflareApiClient.post_logs(any(), any(), any()), return: {:ok, %Tesla.Env{}})
+
+      {:ok, state} = init_with_default(flush_interval: 60_000)
+
+      generate_logs(state, @default_config[:batch_max_size] - 1)
+
+      for e <- BatchCache.pending_events_not_in_flight() do
+        e
+        |> PendingLoggerEvent.changeset(%{api_request_started_at: System.monotonic_time()})
+        |> Repo.update()
+      end
+
+      generate_logs(state, @default_config[:batch_max_size])
+
+      refute_called(LogflareApiClient.post_logs(any(), any(), any()))
+    end
   end
 
   describe "HttpBackend.handle_info/2" do
@@ -82,6 +93,13 @@ defmodule LogflareLogger.HttpBackendTest do
     end
   end
 
+  defp generate_logs(state, count) do
+    for i <- 1..count do
+      msg = {:info, nil, {Logger, "log message", ts(i), []}}
+      {:ok, _state} = HttpBackend.handle_event(msg, state)
+    end
+  end
+
   defp init_with_default() do
     HttpBackend.init(HttpBackend, @default_config)
   end