The approach of doing a recursive function call to expand the if checks
for known metadata names was tripping up an optimization clang has to
collapse that if/then tree into an optimized tree search over the set of
known strings. By unrolling that loop (with a code generator) we start
to present a pattern that clang *can* recognize, and hopefully get some
more stable and faster code generation as a benefit.
<!--
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>
On endpoint shutdown, the `AsyncIOState` object was sometimes freed
before the last async read callback execution had run. This resulted in
callback handlers attempting to access a deleted `io_state_` object.
This PR removes the AsyncIOState dependency from the user callback
execution step, since only the callback and its status are needed at
that point.
Previous crashes were non-deterministic, but would reliably pop up once
on most runs of:
```
bazel test --cxxopt='/DEBUG=full' --config=windows_dbg --test_filter='CoreEnd2endTest*' \
--flaky_test_attempts=1 --cache_test_results=no \
--test_env=GRPC_VERBOSITY=debug --test_env=GRPC_TRACE=event_engine \
//test/core/end2end:core_end2end_tests@experiment=event_engine_client```
---------
Co-authored-by: drfloob <drfloob@users.noreply.github.com>
Reverts grpc/grpc#33223. Breaks internal build.
```
work_stealing_thread_pool.cc:180:27: error: format specifies type 'double' but the argument has type 'gpr_cycle_counter' (aka 'long') [-Werror,-Wformat]
```
We had an infinite recursion in our tracer parsing code... probably not
a biggy, but it was blocking the fuzzers so fixing.
<!--
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 hack to get around an issue on Apple devices caused by the
PosixEventEngine's `poll` poller not supporting fork. This PR disables
the EventEngine poller entirely in Python builds. It will therefore
prevent the release of the EventEngine generally, and prevent any
testing of EventEngine integration with gRPC Python, until the `poll`
poller is fixed.
This is another attempt to add support for vsock in grpc since previous
PRs(#24551, #21745) all closed without merging.
The VSOCK address family facilitates communication between
virtual machines and the host they are running on.
This patch will introduce new scheme: [vsock:cid:port] to
support VSOCK address family.
Fixes#32738.
---------
Signed-off-by: Yadong Qi <yadong.qi@intel.com>
Co-authored-by: AJ Heller <hork@google.com>
Co-authored-by: YadongQi <YadongQi@users.noreply.github.com>
For the details of the issue, please see #33157.
Add several paddings in struct grpc_comletion_quete to avoid the false
sharing.
I see performance improvement with the QPS tests.
cpp_protobuf_async_streaming_qps_unconstrained_1cq_insecure: ~2.9%
cpp_protobuf_async_unary_qps_unconstrained_1cq_insecure: ~7%
<!--
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.
-->
~5% flake introduced earlier this week, where the pool was not scaling
when draining work @ shutdown. Maintaining a busy thread count at
shutdown allows the lifeguard to notice when all threads are blocked,
and spawn new threads if necessary.
We were previously trying to adjust the initial guess flow control
window *and* the adjustment methodology, which created some problems.
The latter is more important for us to get in, so don't change the
former for now (we can do that later).
<!--
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.
-->
recv_initial_metadata_ in `RetryFilter::CallAttempt` is deleted before
it's accessed in
`ClientChannel::FilterBasedLoadBalancedCall::RecvTrailingMetadataReady`
- instead of accessing it, pull out peer string and keep a ref
- switch to json_object_loader for config parsing
- use `absl::string_view` instead of `const char*` for cert provider
names
- change cert provider registry to use a map instead of a vector
- remove unused mesh_ca cert provider factory
I generated a new client key and cert where a Spiffe ID is added as the
URI SAN. As such, we are able to test the audit log contains the
principal correctly.
Update: I switched to use the test logger to verify the log content and
removed stdout logger here because one the failure of [RBE Windows Debug
C/C++](https://source.cloud.google.com/results/invocations/c3187f41-bb1f-44b3-b2b1-23f38e47386d).
Update again: Refactored the test logger in a util such that the authz
engine test also uses the same logger. Subsequently, xDS e2e test will
also use it.
---------
Co-authored-by: rockspore <rockspore@users.noreply.github.com>
The PR does the following:
* Splits the single experiments.yaml file into two files:
experiments.yaml and rollouts.yaml.
* The experiments.yaml will now only include experiment definitions. The
default values of the experiments must now be specified in rollouts.yaml
* Removes the 'release' default value because it is not used.
* Adds an additional_constraints character string to ExperimentMetadata.
* Introduces a hook in src/core/lib/experiments/config.h to allow
registering arbitrary experiment constraint validation callbacks. These
callbacks would take an ExperimentMetadata object as input and return
the correct value to use for an experiment subject to additional
constraints.
Parties prefer to wakeup inline, however there are some mechanisms that
want an out-of-line wakeup (say due to a previously held mutex that may
be re-taken). To help those cases permit a guaranteed asynchronous
wakeup.
(needed now for resolver wakeups on the client call path)
We defaulted this on 5 months ago, and it seems to be working... let's
remove the experiment bit!
---------
Co-authored-by: ctiller <ctiller@users.noreply.github.com>
- Accept JSON null for any optional field.
- Do *not* accept JSON null for wrapper types (`absl::optional<>`,
`std::unique_ptr<>`, and `RefCountedPtr<>`) that are *not* marked as
optional fields.
(This is a re-open PR for https://github.com/grpc/grpc/pull/32999, which
was closed accidentally due to the branch re-base and force-push)
Implement the frame serialization/deserialization method in chaotic-good
transport.
Previous comments from Craig:
- Since messages are not part of the framing system anymore, I think we
should remove ReceiveMessage (and therefore ReceivePadding) from this
type.
(instead we should add some helper functions to get the message lengths)
-- Resolved
- This approach will cause all frame manipulation code to know about
this serialization detail, rather than just the code that's serializing
it - I think it would be better to keep the type, flags separation (even
if we need to change the flags representation)
-- Done, changed back to type, flags separation.
<!--
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.
-->
I have not been able to reproduce the non-empty pool @ shutdown bug in
around 200k runs of various kinds. Now that experiments are marked flaky
by default, any similar failures should not block PR submission, and
this will give me good signal if the bugs reproduce more frequently in
the CI environment.
I have a fix in theory, but I don't think it should be necessary. If the
bug reproduces, I'll try the fix.
This brings the `ThreadPoolTest.ForkStressTest` down from ~4s to < 1s,
by reducing the lower bound on worker thread wait time between fork
checks when no work is available.
Also changed lifetime of lifeguard's `thread_running_`. I don't believe
this fixes the rare TSAN race we've been seeing, but I believe this
range is more correct.
Make the change to improve branch prediction.
For the details, please see #33204.
With the change, the QPS increases by ~0.5% in
"cpp_protobuf_sync_streaming_qps_unconstrained_10mps_secure" test.
cpp_protobuf_sync_streaming_qps_unconstrained_10mps_secure: +0.515%
Most of these data structures need to scale a bit like per-cpu, but not
entirely. We can have more than one cpu hit the same instance in most
cases, and probably want to cap out before the hundreds of shards some
platforms have.
<!--
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 the changes made to ring_hash in #29872. The comment in the
picker code specifically says not to change these variables to an
unsigned type, but that's exactly what that PR did. I don't know if this
has actually been causing any problems, but given that we duplicated
this algorithm (and that comment) from elsewhere without doing a
detailed analysis of it, it seems prudent to stick with the types that
the original code suggested were important.
To avoid causing problems for ObjC, I have changed this such that
ring_hash is not built unless we are building with xDS support, which we
exclude on mobile. Currently, there is no way to use ring_hash without
xDS, although that might change in the future; if it does, we can deal
with any problems that arise at that point.
Upgrade apple platform deployment_target versions to fix the cocoapods
push of BoringSSL-GRPC about the following error:
```
ld: file not found: /Applications/Xcode.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/lib/arc/libarclite_macosx.a
clang: error: linker command failed with exit code 1 (use -v to see invocation)
```
ref: https://developer.apple.com/forums/thread/725300
This also aligns with the versions required by
[protobuf](https://github.com/protocolbuffers/protobuf/pull/10652)
```
ios.deployment_target = '10.0'
osx.deployment_target = '10.12'
tvos.deployment_target = '12.0'
watchos.deployment_target = '6.0'
```
This test mode tries to create threads wherever it legally can to
maximize the chances of TSAN finding errors in our codebase.
<!--
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>
Without this fix it can be the case in the filter based stack that a
unary request that sees the response payload from the wire, prior to
being cancelled, can see the response payload even though we report
cancelled - since we unilaterally remove the error status from the batch
op just prior to evaluating whether to scrub the payload or not.
(also tweaks a couple of log lines that were useful in diagnosing this
bug)
AJ Heller
Craig Tiller
Xuan Wang
Mark D. Roth
apolcyn
Vignesh Babu
Yijie Ma
Yadong
Steven Wang
Luwei Ge
nanahpang
Hannah Shi