Skip to content

[BUG] Elasticsearch async exporter counts a session's completion zero or twice #4338

Description

@thc1006

Describe your environment

main at 11fa0db0, Linux, GCC 14.2, -DWITH_ELASTICSEARCH=ON -DWITH_ASYNC_EXPORT_PREVIEW=ON. AsyncResponseHandler only exists under ENABLE_ASYNC_EXPORT.

Steps to reproduce

Two exports through a fake HttpClient injected via the public constructor. The first session reports two terminal events, which is what the curl path does on a setup failure (see below). The second never calls back.

int session = 0;
auto client = std::make_shared<FakeHttpClient>([&session](http_client::EventHandler &handler) {
  if (++session == 1)
  {
    handler.OnEvent(http_client::SessionState::ConnectFailed, "");
    handler.OnEvent(http_client::SessionState::CreateFailed, "");
  }
  // session 2 never calls back
});

ExportOnce(exporter);  // session 1
ExportOnce(exporter);  // session 2, still in flight

const bool flushed = exporter.ForceFlush(std::chrono::milliseconds{20});

What is the expected behavior?

flushed == false. One of the two sessions has not finished.

What is the actual behavior?

[repro] ForceFlush with one session still running returned true

Immediately, in 0 ms. finished_session_counter_ reached 2 from a single session, so finished >= running was already true.

I ran this on top of the fix in #4337, so this is not the timeout defect reported in #4336. The counters are monotonic and compared with >=, which makes the overshoot permanent: every later flush also returns one session early.

Additional context

AsyncResponseHandler accounts a session's completion neither at most once nor at least once.

More than once. OnResponse and every handled terminal OnEvent call result_callback_ directly, and the callback increments finished_session_counter_. There is no guard. Two in-tree paths produce two terminal callbacks for one session:

  • HttpOperation::SendAsync dispatches ConnectFailed and returns non-CURLE_OK when Setup() fails (ext/src/http/client/curl/http_operation_curl.cc, the code != CURLE_OK branch). Session::SendRequest then takes its else branch and dispatches CreateFailed for the same handler (ext/src/http/client/curl/http_client_curl.cc). This is deterministic, not a race.
  • The async completion lambda in Session::SendRequest uses two independent if blocks rather than else if, so an operation that was aborted and also has a response fires OnEvent(Cancelled) and OnResponse.

Fewer than once. ReadError, WriteError and Destroyed fall into default: break, so result_callback_ never fires and the session is never counted as finished. The synchronous handler treats all three as terminal, Destroyed since #4298 and the other two in #4331, so the two handlers in the same file currently disagree.

Suggested shape

Route every outcome through one idempotent step:

void CompleteOnce(sdk::common::ExportResult result) noexcept
{
  bool expected = false;
  if (completed_.compare_exchange_strong(expected, true, std::memory_order_acq_rel))
  {
    result_callback_(result);
  }
}

with OnResponse and every terminal OnEvent going through it, and the switch enumerating all fifteen states so -Wswitch catches a future addition rather than a default: swallowing it. The guard is also what makes widening the terminal set safe, since it removes the double-completion objection to treating ReadError, WriteError and Destroyed as terminal here the way the synchronous handler does.

Worth covering: each of the three ignored states finishing one session as a failure, a response followed by another terminal event counting once, a cancellation followed by a late response counting once, and a duplicate terminal event counting once.

Two things I would rather flag than fold in. The two if blocks and the non-returning gzip failure branch in Session::SendRequest are shared HTTP client behaviour, so fixing them there would remove the double-fire for every consumer, but that is a separate change. And an exporter side guard is still warranted either way, because EventHandler is a public extension point and an injected client is free to emit whatever it likes.

I am happy to open the PR for the exporter side. It overlaps #4297 and #4337 in the same file, so I would rather land those first unless you would prefer it sooner.

Activity

  1. added
    needs-triageIndicates an issue or PR lacks a `triage/foo` label and requires one.
    on Aug 2, 2026
  2. github-actions commented on Oct 4, 2026

    @github-actions

    This issue was marked as stale due to lack of activity.

  3. thc1006 commented on Oct 24, 2026

    @thc1006
    MemberAuthor

    Still live on main at ac7b87a8, and it now sits in front of a pull request that is waiting on review, so it is worth taking the stale label off.

    The reproduction in the description still holds. What I did not have in August is what the duplicate costs once ForceFlush starts trusting the counter. #4523 changes the return from timeout_steady > 0 to finished_session_counter_ >= running_counter, which is the right question, and the over-count answers it with the wrong export. Two exports, the first delivering ConnectFailed then CreateFailed, the second accepted and never answered, three runs per cell, correct answer false everywhere:

    two terminal events control, one terminal event
    main ac7b87a8 true at 0 ms true at 1000 ms
    #4523 true at 0 ms false at 5000 ms
    #4502 true at 1000 ms true at 1000 ms
    #4523 plus #4502 false at 5000 ms false at 5000 ms

    The control row is what isolates this issue: with a single terminal event #4523 waits out the deadline and answers honestly, so the duplicate is the only difference between those two cells. #4502 on its own closes the duplicate, which is why its two cells agree, and it still reports true because it carries main's return.

    The production halves merge without conflict; only the changelog and the test file collide.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    Staleneeds-triageIndicates an issue or PR lacks a `triage/foo` label and requires one.

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions