fix: harden logtail push lifecycle#25645
Conversation
Qodo reviews are paused for this user.Troubleshooting steps vary by plan Learn more → On a Teams plan? Using GitHub Enterprise Server, GitLab Self-Managed, or Bitbucket Data Center? |
XuPeng-SH
left a comment
There was a problem hiding this comment.
Re-reviewed the latest head be0fc6d627. The only PR-local delta after my last pass is the mock logtail session lifecycle fix in pkg/vm/engine/test/testutil: testClientSession now has a real session context rooted in the transport context, Close() cancels it, and Write() terminates on either the caller context or the session/transport shutdown. That closes the remaining test-fixture gap where a mock session could keep looking alive and keep a send path blocked after transport teardown, so the unhappy-path tests now model the production morpc/session lifetime much more faithfully.
I reran the focused packages on the latest head (./pkg/vm/engine/test/testutil, ./pkg/vm/engine/tae/logtail/service, ./pkg/vm/engine/disttae) plus race for ./pkg/vm/engine/test/testutil and ./pkg/vm/engine/tae/logtail/service, and I do not see a new high-confidence correctness or Q1/Q2/Q3 blocker on this head. GitHub still will not let me APPROVE my own pull request, so I am leaving this as another approve-level review comment.
gouhongshen
left a comment
There was a problem hiding this comment.
Review result: REQUEST_CHANGES. I reviewed the lifecycle, ordering, cancellation, callback ownership, slow-session isolation, heartbeat/bootstrap protocol, and regression tests. I found one P1 blocker below. The issue is a protocol-ordering violation under a congested session, so this cannot be approved as-is. Validation: git diff --check passed; the focused logstore/sm unit and race tests passed. The service/disttae package tests could not compile in this checkout because the environment lacks required cgo headers (usearch.h/roaring.h/xxhash.h).
What type of PR is this?
Which issue(s) this PR fixes:
Fixes #25644
What this PR does / why we need it:
Hardens the TN logtail push lifecycle under normal unhappy-path conditions.
Validation
go test -race ./pkg/vm/engine/tae/logtail/service ./pkg/vm/engine/tae/logtail ./pkg/vm/engine/disttaegit diff --check