This bug occurred when the same xDS server was configured twice in the
same bootstrap config, once in an authority and again as the top-level
server. In that case, we were incorrectly failing to de-dup them and
were creating a separate channel for the LRS stream than the one that
already existed for the ADS stream. We fix this by canonicalizing the
server keys the same way in both cases.
As a separate follow-up item, I will work on trying to find a better way
to key these maps that does not suffer from this kind of fragility.
Expand server promises to run with C++ end2end tests.
Across connected_channel/call/batch_builder/pipe/transport:
- fix a bug where read errors weren't propagated from transport to call
so that we can populate failed_before_recv_message for the c++ bindings
- ensure those errors are not, however, used to populate the returned
call status
Add a new latch call arg to lazily propagate the bound CQ for a server
call (and client call, but here it's used degenerately - it's always
populated). This allows server calls to be properly bound to
pollsets.(1)/(2)
In call.cc:
- move some profiling code from FilterStackCall to Call, and then use it
in PromiseBasedCall (this should be cleaned up with tracing work)
- implement GetServerAuthority
In server.cc:
- use an RAII pattern on `MatchResult` to avoid a bug whereby a tag
could be dropped if we cancel a request after it's been matched but
before it's published
- fix deadline export to ServerContext
In resource_quota_server.cc:
- fix some long standing flakes (that were finally obvious with the new
test code) - it's legal here to have client calls not arrive at the
server due to resource starvation, work through that (includes adding
expectations during a `Step` call, which required some small tweaks to
cq_verifier)
In the C++ end2end_test.cc:
- strengthen a flaky test so it passes consistently (it's likely we'll
revisit this with the fuzzing efforts to strengthen it into an actually
robust test)
(1) It's time to remove this concept
(2) Surprisingly the only test that *reliably* demonstrates this not
being done is time_change_test
---------
Co-authored-by: ctiller <ctiller@users.noreply.github.com>
This reverts commit 4b46dbc19e.
Reason: this seems to be breaking load reports in certain cases,
b/276944116
Let's revert so this doesn't accidentally get released.
<!--
If you know who should review your pull request, please assign it to
that
person, otherwise the pull request would get assigned randomly.
If your pull request is for a specific language, please add the
appropriate
lang label.
-->
<!--
If you know who should review your pull request, please assign it to
that
person, otherwise the pull request would get assigned randomly.
If your pull request is for a specific language, please add the
appropriate
lang label.
-->
---------
Co-authored-by: ctiller <ctiller@users.noreply.github.com>
This reverts commit 7bd9267f32.
<!--
If you know who should review your pull request, please assign it to
that
person, otherwise the pull request would get assigned randomly.
If your pull request is for a specific language, please add the
appropriate
lang label.
-->
<!--
If you know who should review your pull request, please assign it to
that
person, otherwise the pull request would get assigned randomly.
If your pull request is for a specific language, please add the
appropriate
lang label.
-->
This is a big rewrite of global config.
It does a few things, all somewhat intertwined:
1. centralize the list of configuration we have to a yaml file that can
be parsed, and code generated from it
2. add an initialization and a reset stage so that config vars can be
centrally accessed very quickly without the need for caching them
3. makes the syntax more C++ like (less macros!)
4. (optionally) adds absl flags to the OSS build
This first round of changes is intended to keep the system where it is
without major changes. We pick up absl flags to match internal code and
remove one point of deviation - but importantly continue to read from
the environment variables. In doing so we don't force absl flags on our
customers - it's possible to configure grpc without the flags - but
instead allow users that do use absl flags to configure grpc using that
mechanism. Importantly this lets internal customers configure grpc the
same everywhere.
Future changes along this path will be two-fold:
1. Move documentation generation into the code generation step, so that
within the source of truth yaml file we can find all documentation and
data about a configuration knob - eliminating the chance of forgetting
to document something in all the right places.
2. Provide fuzzing over configurations. Currently most config variables
get stashed in static constants across the codebase. To fuzz over these
we'd need a way to reset those cached values between fuzzing rounds,
something that is terrifically difficult right now, but with these
changes should simply be a reset on `ConfigVars`.
<!--
If you know who should review your pull request, please assign it to
that
person, otherwise the pull request would get assigned randomly.
If your pull request is for a specific language, please add the
appropriate
lang label.
-->
---------
Co-authored-by: ctiller <ctiller@users.noreply.github.com>
We currently take named metrics recorded per-request only. Instead we
should merge field-wise.
<!--
If you know who should review your pull request, please assign it to
that
person, otherwise the pull request would get assigned randomly.
If your pull request is for a specific language, please add the
appropriate
lang label.
-->
<!--
If you know who should review your pull request, please assign it to
that
person, otherwise the pull request would get assigned randomly.
If your pull request is for a specific language, please add the
appropriate
lang label.
-->
<!--
If you know who should review your pull request, please assign it to
that
person, otherwise the pull request would get assigned randomly.
If your pull request is for a specific language, please add the
appropriate
lang label.
-->
This test is flaky only with iomgr, this fix will likely fix this.
<!--
If you know who should review your pull request, please assign it to
that
person, otherwise the pull request would get assigned randomly.
If your pull request is for a specific language, please add the
appropriate
lang label.
-->
The `XdsFaultInjectionMaxFault` test has seen a few flakes since #32326
was merged. I believe the flakiness is caused by the fact that when a
large number of RPCs are queued up before the resolver result comes in,
those RPCs are now re-processed in parallel instead of sequentially,
which can cause us to delay more RPCs than we should due to the
`max_faults` setting. To fix this, we change the test to ensure that the
channel is connected (i.e., the resolver result has already been
returned) before we start sending a large number of concurrent RPCs.
Although this is the only test that I've seen flakes in, I've made this
same change consistently to all fault injection tests that are creating
a large number of concurrent RPCs, since the same flake could affect any
of them.
These tests are failing because they're running with too few threads,
however if we give them sufficient threads to catch bugs they're flaky.
Remove them and get the team some bandwidth back.
* Revert "Revert "Revert "Revert "server: introduce ServerMetricRecorder API and move per-call reporting from a C++ interceptor to a C-core filter (#32106)" (#32272)" (#32279)" (#32293)"
This reverts commit 1f960697c5.
* Do not create CallMetricRecorder if call is null.
* Revert "Revert "server: introduce ServerMetricRecorder API and move per-call reporting from a C++ interceptor to a C-core filter (#32106)" (#32272)"
This reverts commit deb1e25543.
* Fix by caching call metric recording stuff in async request
PR #32106 caused msan errors in some tests while de-referencing the
server object where async calls are active after the server is
destroyed. Instead cache the ServerMetricRecorder pointer.
* copyright headers fixed
* clang fixes.
There was a ~1% flake in grpclb end2end tests that was reproducible in opt builds, manifesting as a hang, usually in a the SingleBalancerTest.Fallback test. Through experimentation, I found that by skipping the death test in the grpclb end2end test suite, the hang was no longer reproducible in 10,000 runs. Similarly, moving this test to the end of the suite, or making it run first (as is the case in this PR) resulted in 0 failures in 3000 runs.
It's unclear to me yet why the death test causes things to be unstable in this way. It's clear from the logs that one test does affect the rest, grpc_init is done once for all tests, so all tests utilize the same EventEngine ... until the death test completes, and a new EventEngine is created for the next test.
I think this death test is sufficiently artificial that it's fine to change the test ordering itself, and ignore the wonky intermediate state that results from it.
Reproducing the flake:
```
tools/bazel --bazelrc=tools/remote_build/linux.bazelrc test \
-c opt \
--test_env=GRPC_TRACE=event_engine \
--runs_per_test=5000 \
--test_output=summary \
test/cpp/end2end/grpclb_end2end_test@poller=epoll1
```
* WRR: port StaticStrideScheduler to OSS
* WIP
* Automated change: Fix sanity tests
* fix build
* remove unused aliases
* fix another type mismatch
* remove unnecessary include
* move benchmarks to their own file, and don't run it on windows
* Automated change: Fix sanity tests
* add OOB reporting
* generate_projects
* clang-format
* add config parser test
* clang-tidy and minimize lock contention
* add config defaults
* add oob_reporting_period config field and add basic test
* Automated change: Fix sanity tests
* fix test
* change test to use basic RR
* WIP: started exposing peer address to LB policy API
* first WRR test passing!
* small cleanup
* port RR fix to WRR
* test helper refactoring
* more test helper refactoring
* WIP: trying to fix test to have the right weights
* more WIP -- need to make pickers DualRefCounted
* fix timer ref handling and get tests working
* clang-format
* iwyu and generate_projects
* fix build
* add test for OOB reporting
* keep only READY subchannels in the picker
* add file missed in a previous commit
* fix sanity
* iwyu
* add weight expiration period
* add tests for weight update period and OOB reporting period
* Automated change: Fix sanity tests
* lower bound for timer interval
* consistently apply grpc_test_slowdown_factor()
* cache time in test
* add blackout_period tests
* avoid some unnecessary copies
* clang-format
* add field to config test
* simplify orca watcher tracking
* attempt to fix build
* iwyu
* generate_projects
* update xds proto dependency
* add xDS LB policy entry to registry
* add "_experimental" suffix to policy name
* update LB policy name and remove debug log
* add env var protection
* generate_projects
* gen_upb_api
* WRR: update tests to cover qps plumbing
* WIP
* Automated change: Fix sanity tests
* more WIP
* basic WRR e2e test working
* add OOB test
* add xDS WRR e2e test
* clang-format
* fix sanity
* ignore duplicate addresses
* Automated change: Fix sanity tests
* add new tracer to doc/environment_variables.md
* retain scheduler state across pickers
* Automated change: Fix sanity tests
* use separate mutexes for scheduler and timer
* sort addresses to avoid index churn
* remove fetch_sub for wrap around in RR case
Co-authored-by: markdroth <markdroth@users.noreply.github.com>
* WRR: port StaticStrideScheduler to OSS
* WIP
* Automated change: Fix sanity tests
* fix build
* remove unused aliases
* fix another type mismatch
* remove unnecessary include
* move benchmarks to their own file, and don't run it on windows
* Automated change: Fix sanity tests
* add OOB reporting
* generate_projects
* clang-format
* add config parser test
* clang-tidy and minimize lock contention
* add config defaults
* add oob_reporting_period config field and add basic test
* Automated change: Fix sanity tests
* fix test
* change test to use basic RR
* WIP: started exposing peer address to LB policy API
* first WRR test passing!
* small cleanup
* port RR fix to WRR
* test helper refactoring
* more test helper refactoring
* WIP: trying to fix test to have the right weights
* more WIP -- need to make pickers DualRefCounted
* fix timer ref handling and get tests working
* clang-format
* iwyu and generate_projects
* fix build
* add test for OOB reporting
* keep only READY subchannels in the picker
* add file missed in a previous commit
* fix sanity
* iwyu
* add weight expiration period
* add tests for weight update period and OOB reporting period
* Automated change: Fix sanity tests
* lower bound for timer interval
* consistently apply grpc_test_slowdown_factor()
* cache time in test
* add blackout_period tests
* avoid some unnecessary copies
* clang-format
* add field to config test
* simplify orca watcher tracking
* attempt to fix build
* iwyu
* generate_projects
* add "_experimental" suffix to policy name
* WRR: update tests to cover qps plumbing
* WIP
* more WIP
* basic WRR e2e test working
* add OOB test
* fix sanity
* ignore duplicate addresses
* Automated change: Fix sanity tests
* add new tracer to doc/environment_variables.md
* retain scheduler state across pickers
* Automated change: Fix sanity tests
* use separate mutexes for scheduler and timer
* sort addresses to avoid index churn
* remove fetch_sub for wrap around in RR case
Co-authored-by: markdroth <markdroth@users.noreply.github.com>
* Revert "Revert "xDS stateful session affinity: add config plumbing (#31827)" (#31873)"
This reverts commit 4f15d3dcf9.
* fix build for compilers too dumb to recognize the full set of enum values
Mark D. Roth