Skip to content

Enabled GPR_ABSEIL_SYNC#23372

Merged
veblush merged 1 commit intogrpc:masterfrom
veblush:on-GPR_ABSEIL_SYNC
Jul 28, 2020
Merged

Enabled GPR_ABSEIL_SYNC#23372
veblush merged 1 commit intogrpc:masterfrom
veblush:on-GPR_ABSEIL_SYNC

Conversation

@veblush
Copy link
Copy Markdown
Contributor

@veblush veblush commented Jul 1, 2020

Since all major requirements for enabling GPR_ABSEIL_SYNC are merged, it's time to turn this on. Notably two things were addressed;

  1. Hardening the Linux build environments for wrapped languages. In a nutshell, all Linux build systems are now leveraging manylinux2010 or its variants to build most compatible artifacts. Ruby was the last one to employ this. (Ruby docker image based on manylinux2010 #23344).

  2. Making exit code of tests more strict about joining the gRPC thread. Because Abseil uses a global variable in implementing its locks, it became more susceptible to MSAN because existing gRPC tests used to leave the gRPC threads running at the end of the test. These had to be modified like the main test function waiting until all the gRPC threads terminate. (Added call to grpc::testing::TestEnvironment in tests #23378)

Now all lock implementation on gRPC starts to use Abseil by default. Once this is proven safe via the upcoming releases, all gRPC-specific lock implementation will be removed and Abseil will be used at all times.

@veblush veblush added the release notes: no Indicates if PR should not be in release notes label Jul 1, 2020
@veblush veblush force-pushed the on-GPR_ABSEIL_SYNC branch 2 times, most recently from c935d28 to a5dd172 Compare July 1, 2020 05:33
@veblush
Copy link
Copy Markdown
Contributor Author

veblush commented Jul 1, 2020

grpc_build_artifacts_multiplatform: passed except ruby

@veblush
Copy link
Copy Markdown
Contributor Author

veblush commented Jul 1, 2020

With #23378, msan failures became better and remaining failures are

  • //test/core/end2end:h2_proxy_nosec_test@empty_batch@poller=epoll1
  • //test/cpp/microbenchmarks:bm_callback_streaming_ping_pong@poller=epollex
  • //test/cpp/microbenchmarks:bm_cq@poller=epoll1
  • //test/cpp/microbenchmarks:bm_cq@poller=epollex
  • //test/cpp/microbenchmarks:bm_error
  • //test/cpp/microbenchmarks:bm_fullstack_streaming_ping_pong@poller=epollex
  • //test/cpp/server:server_builder_with_socket_mutator_test@poller=poll
  • //test/cpp/util:slice_test

@veblush veblush force-pushed the on-GPR_ABSEIL_SYNC branch 2 times, most recently from 0eb934d to 8260c25 Compare July 1, 2020 23:48
@veblush
Copy link
Copy Markdown
Contributor Author

veblush commented Jul 1, 2020

With #23379, msan failures are all gone but chances are that h2 failures can come back since I haven't made any change on them.

@veblush veblush force-pushed the on-GPR_ABSEIL_SYNC branch from 8260c25 to a6b75af Compare July 4, 2020 23:21
@veblush
Copy link
Copy Markdown
Contributor Author

veblush commented Jul 4, 2020

Tested with #23344: grpc_build_artifacts_multiplatform: passed except existing upb failures

@veblush
Copy link
Copy Markdown
Contributor Author

veblush commented Jul 17, 2020

grpc_build_artifacts_multiplatform: passed except existing python errors

@veblush veblush force-pushed the on-GPR_ABSEIL_SYNC branch from dfd96da to 2bc76cf Compare July 18, 2020 22:19
@veblush veblush changed the title [DO-NOT-MERGE] Enabled GPR_ABSEIL_SYNC Enabled GPR_ABSEIL_SYNC Jul 18, 2020
@veblush veblush requested a review from yang-g July 18, 2020 22:36
@veblush veblush marked this pull request as ready for review July 18, 2020 22:36
@veblush veblush added lang/core kind/enhancement release notes: yes Indicates if PR needs to be in release notes and removed release notes: no Indicates if PR should not be in release notes labels Jul 20, 2020
@veblush veblush force-pushed the on-GPR_ABSEIL_SYNC branch 2 times, most recently from 08c198f to 21b36d3 Compare July 21, 2020 20:01
@veblush
Copy link
Copy Markdown
Contributor Author

veblush commented Jul 22, 2020

grpc_build_artifacts_multiplatform: passed

@veblush veblush force-pushed the on-GPR_ABSEIL_SYNC branch from 1338cce to 3ac116f Compare July 28, 2020 03:00
@veblush
Copy link
Copy Markdown
Contributor Author

veblush commented Jul 28, 2020

All failures from Bazel RBE MSAN C/C++ and Bazel RBE Windows Debug C/C++ are not relevant to this.

Copy link
Copy Markdown
Contributor

@yang-g yang-g left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please do a cherry pick import to make sure it does not break internal stuff. Thanks.

@veblush veblush merged commit 68a1222 into grpc:master Jul 28, 2020
@veblush
Copy link
Copy Markdown
Contributor Author

veblush commented Jul 28, 2020

@yang-g Thanks! I already pressed the merge button but I'm running the test internally.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

kind/enhancement lang/core release notes: yes Indicates if PR needs to be in release notes

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants