Skip to content

Click house/v1.54.3#30

Closed
rschu1ze wants to merge 6007 commits intomasterfrom
ClickHouse/v1.54.3
Closed

Click house/v1.54.3#30
rschu1ze wants to merge 6007 commits intomasterfrom
ClickHouse/v1.54.3

Conversation

@rschu1ze
Copy link
Copy Markdown
Member

No description provided.

markdroth and others added 30 commits February 10, 2023 15:04
This test has a race and is flaky, see
[b/268379869](https://b.corp.google.com/issues/268379869). It sends an
RPC to the interop server with 0s `keepaliveTimeout` and expects the
keepalive watchdog timer to fire immediately before the server acks the
ping.

<!--

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.

-->
<!--

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>
Based on a discussion with @yashykt , I outlined the intended use of
`GetDefaultEventEngine`, with a bit of explanation around why we chose
to make it work the way it does.

<!--

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 to get the latest version of clang 15 (15.0.7) for our docker
images based on that. By doing so, I had to address this new git
security enforcement so I added a new file to tame it. In a nutshell,
this PR is about polishing docker images based on clang 15.
The shims assumed that all platforms with Posix socket support would use
the PosixEventEngine. This is not the case for iOS.




<!--

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.

-->
Cleanup and remove ios cpp test cronet

To test manually:
./tools/bazel test //src/objective-c/tests:CppCronetTests

@sampajano 
<!--

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: Sergii Tkachenko <hi@sergii.org>
The set of trace flags is now:
* event_engine
* event_engine_endpoint
* event_engine_endpoint_data: additionally log all sent/received data,
similar to what the shims do.
* event_engine_poller

<!--

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.

-->
Let's see if this fixes the "Bus error" flakes that have been happening
in CI.

If it does, then we can narrow things down a bit. If flakes continue,
then we can revert this PR.
… (#32394)

There were some rollback conflicts, so this isn't a pure rollback.

This reverts commit ba0e55f.




<!--

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.

-->
Struct providers were deprecated
bazelbuild/bazel#7347

release notes: no
…ndencies (#32398)

cc @markdroth 


<!--

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: Jan Tattermusch <jtattermusch@google.com>
Co-authored-by: Sergii Tkachenko <sergiitk@google.com>
Co-authored-by: apolcyn <apolcyn@google.com>
Co-authored-by: Daniel Azuma <dazuma@gmail.com>
Co-authored-by: Yash Tibrewal <yashkt@google.com>
Co-authored-by: Craig Tiller <ctiller@google.com>
Co-authored-by: Esun Kim <veblush@google.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.

-->
While creating an internal CL that depends directly on
tsi_alts_credentials, I was getting linker errors saying ` error:
backward reference detected: grpc_channel_credentials_release`, because
`alts_tsi_handshaker.cc` uses the `grpc_channel_credentials_release`
API, which is defined in the `grpc_security_base` target.
A handful of problems were identified while writing the
WindowsEventEngine Listener. To make the listener review easier, these
fixes can be landed separately.

This is built upon grpc/grpc#32376

Problems that are fixed in this PR:
* `OnConnectCompleted` held a Mutex while calling the user callback,
which can deadlock.
* The WinSocket and some associated data needs to remain alive after the
Endpoint destroyed, since Windows IOCP still needs to use some of that
data. Endpoint destruction and socket shutdown are now decoupled, with
the socket managed by a shared_ptr.

<!--

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: drfloob <drfloob@users.noreply.github.com>
Currently, the peer name is returned with the completion of the
send_initial_metadata op, which does not make sense, because with
retries, we don't actually know the peer name until we complete the
recv_initial_metadata op. This PR changes our code to return the peer
string as an attribute of the recv_initial_metadata op, so that it is
not available to the application until that point. This change may be
user-visible, but since our API docs don't seem to guarantee exactly
when this data will be available, it's not technically a breaking
change.

Note that in the promise-based stack, we were already assuming that the
peer string would be returned as part of the recv_initial_metadata
batch, so this PR helps reduce risk for the promise conversion by making
this semantic change now, thus decoupling it from the promise
conversion.

I have also changed the representation of the string in the metadata
batch to be a `grpc_core::Slice` instead of a `std::string`, so that we
can just take a ref to the string held in the transport instead of
having to copy the whole string for every call.
…ro_setup_test_driver func and fix script_dir (#32405)
…absl::StatusOr<> (#32233)

Removing a number of unused variables. This has no behaviour change. 

These types are not considered "unused variables" by normal
`-Wunused-variable` flags because they have nontrivial destructors, but
these types' destructors are not used for their side effects, so unused
variables of these types should be considered bug-prone.

This PR removes all unused `absl::Status` and `absl::StatusOr<>`
variables I could find in grpc.
This applies to all wrapped languages.




<!--

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.

-->
yousukseung and others added 28 commits March 28, 2023 12:04
<!--

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.

-->
…2744)

Change was created by the release automation script. See
go/grpc-release.
…… (#32785)

…#30024)" (#30147)" (#32461)"

This reverts commit 070ce36.

Co-authored-by: Yijie Ma <yijiem.main@gmail.com>
…813)

This PR aims to de-experimentalize the APIs for GCP Observability. 

We would have ideally wanted public feedback before declaring the APIs
stable, but we need stable APIs for GA.

Changes made after API review with @markdroth @veblush, @ctiller and the
entire Core/C++ team -
* The old experimental APIs `grpc::experimental::GcpObservabilityInit`
and `grpc::experimental::GcpObservabilityClose` are now deprecated and
will be deleted after v.1.55 release.
* The new API gets rid of the Close method and follows the RAII idiom
with a single `grpc::GcpObservability::Init()` call that returns an
`GcpObservability` object, the lifetime of which controls when
observability data is flushed.
* The `GcpObservability` class could in the future add more methods. For
example, a debug method that shows the current configuration.
* Document that GcpObservability initialization and flushing (on
`GcpObservability` destruction) are blocking calls.
* Document that gRPC is still usable if GcpObservability initialization
failed. (Added a test to prove the same).
* Since we don't have a good way to flush stats and tracing with
OpenCensus, the examples required users to sleep for 25 seconds. This
sleep is now part of `GcpObservability` destruction.

Additional Implementation details -
* `GcpObservability::Init` is now marked with `GRPC_MUST_USE_RESULT` to
make sure that the results are used. We ideally want users to store it,
but this is better than nothing.
* Added a note on GCP Observability lifetime guarantees.

<!--

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.

-->
…816)

This reverts commit 4b46dbc.

Reason: this seems to be breaking load reports in certain cases,
b/276944116

Let's revert so this doesn't accidentally get released.
Backport of #32817 

.. as it is now part of the `GcpObservabilityClose()` routine
[itself](https://github.com/grpc/grpc/pull/32715/files#diff-e1ce0ccb4650e20b62a6d6b31655f446ad9ef72efca5849143e65f4a5a5e0310R223).

Background: #32715 may have broken the CI for observability interop
testing. The client seems to be taking too long to finish. More info at
b/277145074

We need to backport this to the `v1.54.x` branch.
Change was created by the release automation script. See
go/grpc-release.
Change was created by the release automation script. See go/grpc-release
…11) (#32916)

<!--

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.

-->

---------

Co-authored-by: ctiller <ctiller@users.noreply.github.com>
Co-authored-by: BrandonY <brandony@gmail.com>
Change was created by the release automation script. See go/grpc-release
Backport #33239 to 1.54.

Co-authored-by: Craig Tiller <ctiller@google.com>
… (#33303)

We configured TestGrid to file bug separately for each failed
sub-target, if we still fail the main target, TestGrid will fail
duplicate bugs.




<!--

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.

-->
…(v1.54.x backport) (#33525)

Backport of #33520 to v1.54.x.
---
Follow up change of #33222.

We don't want file multiple bugs if any of the sub-tests of the
`url_map` test fails.

<!--

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.

-->
PanCakes to the rescue!

We noticed that our 'sanity' test was going to fail, but we think we can
fix that automatically, so we put together this PR to do just that!

If you'd like to opt-out of these PR's, add yourself to NO_AUTOFIX_USERS
in .github/workflows/pr-auto-fix.yaml

Co-authored-by: ctiller <ctiller@users.noreply.github.com>
…v1.54.x backport) (#33725)

Backport of #33699 to v1.54.x.
---
Similar to grpc/grpc-java#8982.
All other languages already to this.

Needed for easier deployment of grpc/grpc#33685.

ref b/274944592
…rt) (#33809)

Backport of #33796 to v1.54.x.
---
grpc/grpc#33699 incorrectly changed the legacy
builds to not just use the test driver from the master, but also to
build from it. This PR fixes the issue, and also updates the python job
to work use the driver from master.
*Beep boop. This is an automatically generated backport of #33738.*

This should resolve breakage on master caused by the jump to Cython
3.0.0 this morning.
Update Phusion base image to address security issues. Original PR:
#33847
@rschu1ze rschu1ze closed this Nov 10, 2023
@CLAassistant
Copy link
Copy Markdown

CLAassistant commented Nov 10, 2023

CLA assistant check
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you all sign our Contributor License Agreement before we can accept your contribution.
1 out of 17 committers have signed the CLA.

✅ rschu1ze
❌ ctiller
❌ ejona86
❌ drfloob
❌ veblush
❌ ananda1066
❌ stanley-cheung
❌ yashykt
❌ jtattermusch
❌ matthewstevenson88
❌ markdroth
❌ apolcyn
❌ yousukseung
❌ gnossen
❌ sergiitk
❌ XuanWang-Amos
❌ eugeneo
You have signed the CLA already but the status is still pending? Let us recheck it.

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.