Skip to content

Update to Envoy Commit c661827 - #1627

Merged
tomjzzhang merged 7 commits into
envoyproxy:mainfrom
fei-deng:envoy-update-20261002
Oct 6, 2026
Merged

tomjzzhang merged 7 commits into
envoyproxy:mainfrom
fei-deng:envoy-update-20261002

Conversation

@fei-deng

@fei-deng fei-deng commented Oct 2, 2026 •

Copy link
Copy Markdown
Contributor
  • Update Envoy Commit

  • Update tests to address compiler warnings, new Envoy metric additions, server log scraping during expected error scenarios, and timing overhead under ThreadSanitizer (TSAN).

…test assertion

Signed-off-by: fei-deng <101672975+fei-deng@users.noreply.github.com>
@fei-deng fei-deng changed the title Update Envoy dependency to 0.5.2.envoy and fix Googletest 1.18 death … Update to Envoy Commit c661827 Oct 2, 2026
- Change strict counter len assertions to assertGreaterEqual to handle Envoy http2 metrics
- Add NO_REQUEST_LINE_IN_REQUEST protocol error to test server warn/error ignore list
- Add 1-request tolerance for timing jitter in test_http_request_release_timing

Signed-off-by: fei-deng <101672975+fei-deng@users.noreply.github.com>
Signed-off-by: fei-deng <101672975+fei-deng@users.noreply.github.com>
Signed-off-by: fei-deng <101672975+fei-deng@users.noreply.github.com>
@fei-deng
fei-deng marked this pull request as ready for review October 3, 2026 15:12
@tomjzzhang
tomjzzhang self-requested a review October 5, 2026 21:22
Comment thread test/integration/test_integration_basics.py Outdated
# the GreaterEqual matchers below.
total_requests = qps_parameterization_fixture * concurrency * math.floor(actual_duration)
# the GreaterEqual matchers below. Allow a 1-request tolerance for timing jitter.
total_requests = max(

@tomjzzhang tomjzzhang Oct 5, 2026 •

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you provide more data around this. I'd like more information to properly audit this claim.

If there is jitter, why would it be restricted to a single request? Couldn't it be for every worker before it fully completes? Do you mind explain the series of events that would lead to this edge scenario?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  1. Floored Duration Baseline: The test measures actual_duration as a float and takes math.floor(actual_duration) (e.g. 5.002s -> 5s). This flooring already establishes a conservative lower bound for expected request generation. In the CI failure logs that prompted this change, the recorded request count was off by exactly 1 request (e.g., 249 recorded vs. 250 expected).

  2. Mid-Run Execution: During the run, thread scheduling jitter under heavy CI load does not drop requests—Nighthawk simply executes them a few milliseconds later, and their completed responses are recorded normally in the histogram.

  3. Shutdown Boundary: The only point where a request can be clipped from the recorded count is at the exact process termination cutoff, where 1 in-flight request may be incomplete when statistics capture stops.
    Subtracting 1 request (- 1) provides a simple, effective global floor tolerance for boundary timing jitter under CI load, ensuring the integration test runs reliably without flaking.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ack. Since we've only observed -1, I think we can keep this test expectation but I think theoretically each worker acts independently and cause a flake although that would be extremely rare.

Comment thread repo.bazelrc Outdated
Comment thread .bazelrc Outdated
Signed-off-by: fei-deng <101672975+fei-deng@users.noreply.github.com>
Signed-off-by: fei-deng <101672975+fei-deng@users.noreply.github.com>
@fei-deng
fei-deng force-pushed the envoy-update-20261002 branch from 0eb2129 to ac6885d Compare October 5, 2026 22:56
tomjzzhang
tomjzzhang previously approved these changes Oct 6, 2026
@tomjzzhang

Copy link
Copy Markdown
Contributor

Can you update the PR description to match the current implementation? Then I'll merge it.

Signed-off-by: fei-deng <101672975+fei-deng@users.noreply.github.com>
@tomjzzhang
tomjzzhang merged commit 6a10e20 into envoyproxy:main Oct 6, 2026
16 of 17 checks passed
@fei-deng
fei-deng deleted the envoy-update-20261002 branch October 6, 2026 16:16
tomjzzhang added a commit to tomjzzhang/nighthawk that referenced this pull request Oct 18, 2026
When envoyproxy#1627 synced .bazelrc with upstream Envoy PR envoyproxy/envoy#47806, it removed try-import %workspace%/repo.bazelrc from the bottom of .bazelrc. However, .github/workflows/_ci.yml still writes the CI compiler and resource configurations to repo.bazelrc, so Bazel no longer loads those settings during CI runs.

This PR replaces repo.bazelrc with user.bazelrc in .github/workflows/_ci.yml (and adds the upstream Envoy fallback in ci/do_ci.sh to copy repo.bazelrc to user.bazelrc if present), as user.bazelrc remains imported at the bottom of .bazelrc and ignored by .gitignore.

Signed-off-by: tomjzzhang <4367421+tomjzzhang@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants