`StatusFlag` acts like a status, but is just a boolean (we don't want to
accidentally treat a boolean as something that indicates failure in case
it's not)
Similarly `ValueOrFailure` looks like `StatusOr` but reduces the failure
space to one value.
---------
Co-authored-by: ctiller <ctiller@users.noreply.github.com>
We're seeing timeout errors in our distribution test:
https://fusion2.corp.google.com/invocations/dfa9aaa9-e94b-479e-8c28-a39d98d277bc/targets/github%2Fgrpc%2Fbuild_artifacts_python;config=default/tests.
Sample error:
`2023-11-10 09:12:19,512 TIMEOUT:
build_artifact.python_windows_x86_Python39_32bit [pid=2320,
time=2700.1sec]`
This change increases timeout for windows build artifact jobs to 7200s,
which aligns with all other jobs (except `linux_extra`, which is 3600s).
<!--
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 commit upgrades gRPC to protobuf v25.0 and makes some fixes to
account for upb changes. One major change is that upb has been merged
into the protobuf repo, so we can now drop the separate `@upb`
dependency. Another is that `.upb.c` files no longer exist and there are
new `.upb_minitable.h` and `.upb_minitable.c` files. The longer
filenames exceeded a Windows restriction, so to work around that I
renamed the `upb-generated` directory to just `upb-gen`, and likewise
for `upbdefs-generated`.
To mitigate the following windows long path issue found in
https://github.com/grpc/grpc/pull/34513. Using `pyb` instead of
`python_build` saves 9 characters.
---
`Distribution Tests Python Windows` failed because of
`T:\altsrc\github\grpc\workspace_python_windows_x86_Python38_32bit\python_build\\temp.win-amd64-cpython-312\\Release\\src\core\ext\upb-gen\envoy\extensions\load_balancing_policies\client_side_weighted_round_robin\v3\client_side_weighted_round_robin.upb_minitable.obj`
This is already present in the grpc python sync stack and has been
missing from aio stack.
CC @gnossen
<!--
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.
-->
So that users can process annotations other than calling ToString().
<!--
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.
-->
Design is documented at
[go/windows-dns-resolver-issue](http://go/windows-dns-resolver-issue)
(note that the design doc is slightly outdated regarding the shared
ownership model of the virtual socket that was implemented in
13bd2b404e).
Passed `//test/cpp/naming:resolver_component_tests_runner_invoker` and
`//test/cpp/naming:cancel_ares_query_test`:
```
C:\Users\yijiem\projects\grpc>bazel --output_base=C:\bazel6 test --dynamic_mode=off --verbose_failures --test_env=GRPC_EXPERIMENTS=event_engine_dns --test_env=GRPC_VERBOSITY=debug --test_env=GRPC_TRACE=cares_resolver --enable_runfiles=yes --nocache_test_results //test/cpp/naming:resolver_component_tests_runner_invoker
INFO: Analyzed target //test/cpp/naming:resolver_component_tests_runner_invoker (1 packages loaded, 8 targets configured).
INFO: Found 1 test target...
INFO: From Compiling src/core/lib/event_engine/windows/windows_engine.cc:
C:\bazel6\execroot\com_github_grpc_grpc\src/core/lib/channel/channel_args.h(287): warning C4312: 'reinterpret_cast': conversion from 'int' to 'void *' of greater size
Target //test/cpp/naming:resolver_component_tests_runner_invoker up-to-date:
bazel-bin/test/cpp/naming/resolver_component_tests_runner_invoker.exe
INFO: Elapsed time: 230.374s, Critical Path: 228.54s
INFO: 9 processes: 2 internal, 7 local.
INFO: Build completed successfully, 9 total actions
//test/cpp/naming:resolver_component_tests_runner_invoker PASSED in 221.2s
Executed 1 out of 1 test: 1 test passes.
```
```
C:\Users\yijiem\projects\grpc>bazel --output_base=C:\bazel6 test --dynamic_mode=off --verbose_failures --test_env=GRPC_EXPERIMENTS=event_engine_dns --test_env=GRPC_VERBOSITY=debug --test_env=GRPC_TRACE=cares_resolver --enable_runfiles=yes --nocache_test_results //test/cpp/naming:cancel_ares_query_test
INFO: Analyzed target //test/cpp/naming:cancel_ares_query_test (0 packages loaded, 0 targets configured).
INFO: Found 1 test target...
Target //test/cpp/naming:cancel_ares_query_test up-to-date:
bazel-bin/test/cpp/naming/cancel_ares_query_test.exe
INFO: Elapsed time: 49.656s, Critical Path: 48.00s
INFO: 6 processes: 2 internal, 4 local.
INFO: Build completed successfully, 6 total actions
//test/cpp/naming:cancel_ares_query_test PASSED in 43.0s
Executed 1 out of 1 test: 1 test passes.
```
<!--
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 current fixed minimum starting thread count (2 threads), combined
with the fixed thread spawn rate limit of 1/sec, was causing test issues
with some new EventEngine integrations.
For example, this test had a race wherein the alarm destructor was
expected to run within 1 second. In 5% of runs, the 2 EventEngine
threads were otherwise occupied, and it would take around 1 second to
spawn a new thread.
1e15d00ec4/test/cpp/common/alarm_test.cc (L418-L435)
Modeled after mutexes in the Rust ecosystem: the mutex owns the data
provided, and acquisition of the mutex returns a handle with which to
manipulate that data.
This fits in nicely with the execution environment we've established
whereby we may want to pass the lock from lambda to lambda for some
time.
---------
Co-authored-by: ctiller <ctiller@users.noreply.github.com>
Add a config to experiments & rollouts to allow dependent experiments to
be flagged.
We're getting past the point where it's possible to reason about which
experiments need to be turned off if we disable some other experiment,
and so this provides some additional rollout safety.
Can be specified in both experiments and rollouts: experiments.yaml
makes the most sense and we should default to it, but rollouts.yaml lets
us put dependencies between internal & external dependencies internally
and that's gonna be a little useful.
---------
Co-authored-by: ctiller <ctiller@users.noreply.github.com>
This is a follow-up PR of #34191, which handles the error condition of
endpoints failed to write/read in chaotic-good client transport.
This PR needs to be merged after #34191.
<!--
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: Bradley Hess <bdhess@google.com>
Co-authored-by: AJ Heller <hork@google.com>
- Fix deadlock in load reporting tests.
- Add timeout to `WaitForLoadReport()`. (Note: this required changing
from `grpc::internal::Mutex` and friends to `grpc_core::Mutex` and
friends.)
- Fix balancer stream shutdown machinery.
- Change `ServerThread` to be a class instead of a struct.
datetime.utcnow() raises a deprecation warning on Python 3.12, so this
change prevents that deprecation warning while also switching to a
monotonic clock for calculating RPC latency
<!--
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.
-->
cc @XuanWang-Amos
Now we log pretty much identical message:
```
client_app.py:320] [psm-grpc-client-7768f6597-nvtgl] Detected successful calls to xDS control plane: trafficdirector.googleapis.com:443
client_app.py:292] [psm-grpc-client-7768f6597-nvtgl] ADS: Detected successful calls to xDS control plane trafficdirector.googleapis.com:443
```
This PR will log the latest channel state in the first message, similar
to what we do in `find_server_channel_with_state`:
52c08f4498/tools/run_tests/xds_k8s_test_driver/framework/test_app/client_app.py (L367-L371)
After the change:
```
client_app.py:320] [psm-grpc-client-6566595cff-8wrfd] Detected successful calls to xDS control plane trafficdirector.googleapis.com:443, channel: <Channel channel_id=4 target=trafficdirector.googleapis.com:443 call_started=9 calls_failed=8 state=READY>
client_app.py:292] [psm-grpc-client-6566595cff-8wrfd] ADS: Detected successful calls to xDS control plane trafficdirector.googleapis.com:443
```
Generating an annotation is often expensive, there's no need to annotate
when not sampled.
<!--
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.
-->
Changes to fake resolver:
- Add `WaitForReresolutionRequest()` method to fake resolver response
generator to allow tests to tell when re-resolution has been requested.
- Change fake resolver response generator API to have only one mechanism
for injecting results, regardless of whether the result is an error or
whether it's triggered by a re-resolution.
Changes to grpclb_end2end_test:
- Change balancer interface such that instead of setting a list of
responses with fixed delays, the test can control exactly when each
response is set.
- Change balancer impl to always send the initial LB response, as
expected by the grpclb protocol.
- Change balancer impl to always read load reports, even if load
reporting is not expected to be enabled. (The latter case will still
cause the test to fail.) Reads are done in a different thread than
writes.
- Allow each test to directly control how many backends and balancers
are started and the client load reporting interval, so that (a) we don't
waste resources starting servers we don't need and (b) there is no need
to arbitrarily split tests across different test classes.
- Add timeouts to `WaitForAllBackends()` functionality, so that tests
will fail with a useful error rather than timing out.
- Improved ergonomics of various helper functions in the test framework.
In the process of making these changes, I found a couple of bugs:
- A bug in pick_first, which I fixed in #34885.
- A bug in grpclb, in which we were using the wrong condition to decide
whether to propagate a re-resolution request from the child policy,
which I've fixed in this PR. (This bug probably originated way back in
#18344.)
This should address a lot of the flakes seen in grpclb_e2e_test
recently.
Roll forward #34657, which was reverted in #34761.
Previous error in CMake:
```
[ RUN ] ClientTransportTest.AddOneStreamMultipleMessages
unknown file: Failure
Unexpected mock function call - returning directly.
Function call: Call(CANCELLED: )
Google Mock tried the following 1 expectation, but it didn't match:
/[var/local/git/grpc/test/core/transport/chaotic_good/client_transport_test.cc:484](https://cs.corp.google.com/piper///depot/google3/var/local/git/grpc/test/core/transport/chaotic_good/client_transport_test.cc?l=484): EXPECT_CALL(on_done, Call(absl::OkStatus()))...
Expected arg #0: is equal to OK
Actual: CANCELLED:
Expected: to be called once
Actual: never called - unsatisfied and active
/[var/local/git/grpc/test/core/transport/chaotic_good/client_transport_test.cc:484](https://cs.corp.google.com/piper///depot/google3/var/local/git/grpc/test/core/transport/chaotic_good/client_transport_test.cc?l=484): Failure
Actual function call count doesn't match EXPECT_CALL(on_done, Call(absl::OkStatus()))...
Expected: to be called once
Actual: never called - unsatisfied and active
real 0.24
user 0.00
sys 0.00
2023-10-20 01:50:32,776 FAILED: cmake/build/client_transport_test --gtest_filter=ClientTransportTest.AddOneStreamMultipleMessages GRPC_POLL_STRATEGY=epoll1 [ret=139, pid=1663532, time=0.3sec]
```
<!--
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 fixes a bug accidentally introduced in #33753. The symptom is that
if we exit idle and then get a new address list before any of the
subchannels in the old list can report their initial connectivity state,
we will incorrectly ignore the new address list.
EventEngine experiments, especially with `work_serializer_dispatch` tend
to cause callbacks to occur later than we've previously seen, so tests
that verify global data structures tend to become flakier when these are
introduced.
Here, the fix is waiting for EventEngine to be closed before starting
the new test.
Whilst here, make some adjustments to the test for better readability on
what's going on:
- if we fail a request to an echo service, we do not actually expect the
messages to match, so don't report that
- if we expect a value of 1 or 2, AnyOf is a better tool: it will report
the actual value too
---------
Co-authored-by: ctiller <ctiller@users.noreply.github.com>
Eliminate spam from clang-format script.
Also add a shuffle to the file list - saves about a second of runtime on
my machine by making sure clusters of hard files are distributed evenly
amongst CPUs.
This adds the directory reloader implementation of the CrlProvider. This
will periodically reload CRL files in a directory per [gRFC
A69](https://github.com/grpc/proposal/pull/382)
Included in this is the following:
* A public API to create the `DirectoryReloaderCrlProvider`
* A basic directory interface in gprpp and platform specific impls for
getting the list of files in a directory (unfortunately prior C++17,
there is no std::filesystem, so we have to have platform specific impls)
* The implementation of `DirectoryReloaderCrlProvider` takes an
event_engine and a directory interface. This allows us to test using the
fuzzing event engine for time mocking, and to implement a test directory
interface so we avoid having to make temporary directories and files in
the tests. This is notably not in `include`, and the
`CreateDirectoryReloaderCrlProvider` is the only way to construct one
from the public API, so we don't expose the event engine and directory
details to the user.
---------
Co-authored-by: gtcooke94 <gtcooke94@users.noreply.github.com>
This PR fixes a bug identified in #29667, where the TLS channel
credentials still require a trust bundle even if the user has explicitly
opted to not verify the server certificate. This PR is based on #29810.
Address https://github.com/grpc/grpc/issues/12554
The API for `duplicate_without_call_credentials` says
```
// Creates a version of the channel credentials without any attached call
// credentials. This can be used in order to open a channel to a non-trusted
// gRPC load balancer.
```
As the impl stands right now, because of that description, in the case
of layered composite creds, I think the right behavior would be to call
down until you get the base cred with no call cred.
In discussing with the team, we do wonder if the use-case of layered
composite creds is really something that should be a feature, or if we
should be checking during the creation of composite creds to make sure
we aren't layering composite creds? @markdroth can you give your
thoughts?
Craig Tiller
gRPC Team Bot
Mark D. Roth
Xuan Wang
Adam Cozzette
Esun Kim
Hima Sajeev
Yousuk Seung
Alisha Nanda
Yijie Ma
AJ Heller
nanahpang
Vignesh Babu
Terry Wilson
Michael P. Nitowski
Sergii Tkachenko
Luwei Ge
Gregory Cooke
apolcyn
Matthew Stevenson
Stanley Cheung