(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.
Will be used to evaluate experiment effects on memory usage once they're
toggled on.
<!--
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.
-->
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)
<!--
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 PR adds lookup cancellation to the client channel resolver fuzzer,
and adds an optional call to `resolver->Orphan()` at a few specific
points.
I also significantly sped up the fuzzer by removing all mutexes, waits,
and sleeps. It's single-threaded, after all.
---------
Co-authored-by: drfloob <drfloob@users.noreply.github.com>
Use an index instead of a string to select tests (and use that index
module total test count to ensure whatever the fuzzer selects we always
run a test).
This will make the fuzzer corpus unstable when the test count changes,
which I think is fine - it'll regenerate.
<!--
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.
-->
ChannelArgs fuzz configuration is expected to be used in other fuzzing
targets as well. This PR extracts the common code from the API fuzzer
and converts to use the C++ types.
Allow for multiple `--grpc_experiments`, `--grpc_trace` command line
arguments to be added, accumulate them, and provide them to gRPC as one
thing.
<!--
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>
Reverts grpc/grpc#33088
The internal test is failing because after the change, transitive
dependencies will be included in response too, thus we need add
`empty2_pb2` to response since it's a dependency of
`empty2_extensions_pb2`.
We already did that in OSS test, we need do the same for internal test
case.
<!--
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 nearly identical to the helloworld example's greeter callback
server, but without the health check service enabled. Having a separate
example for reflection may help folks find this code.
Early out evaluating this function where we can, and use macros to
eliminate function calls in debug builds.
Takes per-example time from 5400ms to 1200ms in debug asan builds.
<!--
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>
<!--
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>
When we deprecate services in proto-file:
```proto
syntax = "proto3";
option php_namespace = "Test";
service MyService {
option deprecated = true;
rpc Call(Test) returns (Test) {
option deprecated = true;
};
}
message Test {
}
```
It might be a good idea to propagate this flat in the generated classes
so that the final users will be aware of the changes coming soon.
Example of the generated code:
```php
<?php
// GENERATED CODE -- DO NOT EDIT!
namespace Test;
/**
* @deprecated
*/
class MyServiceClient extends \Grpc\BaseStub {
/**
* @param string $hostname hostname
* @param array $opts channel options
* @param \Grpc\Channel $channel (optional) re-use channel object
*/
public function __construct($hostname, $opts, $channel = null) {
parent::__construct($hostname, $opts, $channel);
}
/**
* @deprecated
* @param \Test\Test $argument input argument
* @param array $metadata metadata
* @param array $options call options
* @return \Grpc\UnaryCall
*/
public function Call(\Test\Test $argument,
$metadata = [], $options = []) {
return $this->_simpleRequest('/MyService/Call',
$argument,
['\Test\Test', 'decode'],
$metadata, $options);
}
}
```
These tags will cause IDE to highlight the class/method to bring the
developer's attention.
This is consistent with how "protoc" handles the "deprecated" option for
PHP
<!--
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 reverts commit 1624542ea4, relanding
https://github.com/grpc/grpc/pull/32956
Because of some proto dependency and build problems internally, I've
removed the ServiceConfig proto fuzzing component. These build issues
can hopefully be resolved soon, and then we can re-add the deleted
implementation from commit
[b078c9c](b078c9c015)
in this PR.
each oob test runs at least 2s (2 verifications with each server sending
report each 1s), current configuration allows at most 3 clients run in
parallel. In reality, probably only the two clients against one server.
Currently 8 jobs are running each batch, and it happens that each batch
has at most 2 oob clients against one server.
Since 3 languages support orca, so we should allow 3 clients running
against one server so the timeout is at least 4s. We give some buffers
to allow tests running more reliably.
10s in theory support 5-6 clients running against one server.
AJ Heller
nanahpang
Craig Tiller
Steven Wang
Mark D. Roth
Esun Kim
Hannah Shi
Wanlin Du
Eugene Ostroukhov
Yash Tibrewal
Vignesh Babu
Luwei Ge
Xuan Wang
mojh7
Yijie Ma
Mikhail Galanin
Sergii Tkachenko
yifeizhuang