zookeeper: parse server responses by rgs1 · Pull Request #7574 · envoyproxy/envoy

rgs1 · 2019-07-14T15:59:28Z

This adds support for parsing server responses and watch events. One
or more follow-up PRs will add support for latency measurements and
access log.

Signed-off-by: Raul Gutierrez Segales rgs@pinterest.com

This adds support for parsing server respones and watch events. One or more follow-up PRs will add support for latency measurements and access log. Signed-off-by: Raul Gutierrez Segales <rgs@pinterest.com>

Signed-off-by: Raul Gutierrez Segales <rgs@pinterest.com>

rgs1 · 2019-07-14T16:49:17Z

Note: docs & tests still missing, will follow-up later today/tomorrow.

rgs1 · 2019-07-14T17:36:16Z

@mattklein123 got this twice in a row now:

//test/extensions/filters/http/dynamic_forward_proxy:proxy_filter_integration_test TIMEOUT in 315.0s
  /build/tmp/_bazel_bazel/b570b5ccd0454dc9af9f65ab1833764d/execroot/envoy/bazel-out/k8-dbg/testlogs/test/extensions/filters/http/dynamic_forward_proxy/proxy_filter_integration_test/test.log

rgs1 · 2019-07-14T19:30:56Z

/retest

repokitteh-read-only · 2019-07-14T19:31:01Z

🔨 rebuilding ci/circleci: tsan (failed build)

🐱

Caused by: a #7574 (comment) was created by @rgs1.

see: more, trace.

…esponses

Signed-off-by: Raul Gutierrez Segales <rgs@pinterest.com>

…esponses

stale · 2019-07-25T14:25:12Z

This pull request has been automatically marked as stale because it has not had activity in the last 7 days. It will be closed in 7 days if no further activity occurs. Please feel free to give a status update now, ping for review, or re-open when it's ready. Thank you for your contributions!

rgs1 · 2019-07-25T14:27:09Z

I'll push tests today/tomorrow.

…esponses

This should cover most of it, I still need to test some corner cases. Signed-off-by: Raul Gutierrez Segales <rgs@pinterest.com>

Signed-off-by: Raul Gutierrez Segales <rgs@pinterest.com>

rgs1 · 2019-07-28T18:08:51Z

hmm, unrelated failure:

//test/common/runtime:runtime_impl_test                                 TIMEOUT in 315.0s
  /build/tmp/_bazel_bazel/b570b5ccd0454dc9af9f65ab1833764d/execroot/envoy/bazel-out/k8-dbg/testlogs/test/common/runtime/runtime_impl_test/test.log

rgs1 · 2019-07-28T18:09:00Z

/retest

repokitteh-read-only · 2019-07-28T18:09:04Z

🔨 rebuilding ci/circleci: asan (failed build)

🐱

Caused by: a #7574 (comment) was created by @rgs1.

see: more, trace.

rgs1 · 2019-07-28T20:26:46Z

another timeout for the same unrelated integration test:

//test/common/runtime:runtime_impl_test                                 TIMEOUT in 315.0s
  /build/tmp/_bazel_bazel/b570b5ccd0454dc9af9f65ab1833764d/execroot/envoy/bazel-out/k8-dbg/testlogs/test/common/runtime/runtime_impl_test/test.log

cc: @alyssawilk since maybe this is related to e61681d ?

rgs1 · 2019-07-28T20:27:01Z

/retest

repokitteh-read-only · 2019-07-28T20:27:04Z

🔨 rebuilding ci/circleci: asan (failed build)

🐱

Caused by: a #7574 (comment) was created by @rgs1.

see: more, trace.

rgs1 · 2019-07-28T20:32:43Z

Yeah it does time out in the test case added by e61681d:

[----------] 3 tests from StaticLoaderImplTest
[ RUN      ] StaticLoaderImplTest.All
[       OK ] StaticLoaderImplTest.All (95 ms)
[ RUN      ] StaticLoaderImplTest.ProtoParsing
[       OK ] StaticLoaderImplTest.ProtoParsing (147 ms)
[ RUN      ] StaticLoaderImplTest.RuntimeFromNonWorkerThreads
Terminated
-- Test timed out at 2019-07-28 19:03:52 UTC --

This is a follow-up to e61681d. While working on envoyproxy#7574, I am getting consistent timeouts for this test. I think it happens because the code on the main thread runs slower than the code on the reader thread. My fix is to properly acquire and release the lock on each step, to ensure they are properly interlaced. That is: 1) read from reader 2) write from writer 3) read new value from reader 4) read new value from writer To repro the timeout, I added a sleep before step 2. After this change, I can't repro anymore. Signed-off-by: Raul Gutierrez Segales <rgs@pinterest.com>

rgs1 · 2019-07-28T21:02:52Z

Possible fix for timeouts: #7746.

While working on #7574, I am getting consistent timeouts for this test. To repro the timeout, I added a sleep before step 2. After this change, I can't repro anymore. Risk: n/a (test only)

…esponses

snowp

This looks pretty good, just a few comments

source/extensions/filters/network/zookeeper_proxy/decoder.cc

source/extensions/filters/network/zookeeper_proxy/filter.cc

source/extensions/filters/network/zookeeper_proxy/decoder.cc

test/extensions/filters/network/zookeeper_proxy/filter_test.cc

source/extensions/filters/network/zookeeper_proxy/decoder.cc

Signed-off-by: Raul Gutierrez Segales <rgs@pinterest.com>

rgs1 · 2019-07-31T17:01:00Z

/retest

repokitteh-read-only · 2019-07-31T17:01:06Z

🔨 rebuilding ci/circleci: compile_time_options (failed build)

🐱

Caused by: a #7574 (comment) was created by @rgs1.

see: more, trace.

snowp

One minor comment otherwise LGTM

source/extensions/filters/network/zookeeper_proxy/decoder.cc

Signed-off-by: Raul Gutierrez Segales <rgs@pinterest.com>

snowp

LGTM, thanks!

zookeeper: parse server responses

72c6df5

This adds support for parsing server respones and watch events. One or more follow-up PRs will add support for latency measurements and access log. Signed-off-by: Raul Gutierrez Segales <rgs@pinterest.com>

rgs1 requested a review from snowp as a code owner July 14, 2019 15:59

snowp self-assigned this Jul 14, 2019

Fix format

386bc22

Signed-off-by: Raul Gutierrez Segales <rgs@pinterest.com>

Raul Gutierrez Segales added 4 commits July 14, 2019 16:09

Merge remote-tracking branch 'upstream/master' into zk-filter-parse-r…

6e39023

…esponses

Merge remote-tracking branch 'upstream/master' into zk-filter-parse-r…

7bcde20

…esponses

Kick CI

2f723d2

Signed-off-by: Raul Gutierrez Segales <rgs@pinterest.com>

Merge remote-tracking branch 'upstream/master' into zk-filter-parse-r…

328c5de

…esponses

stale bot added the stale stalebot believes this issue/PR has not been touched recently label Jul 25, 2019

stale bot removed the stale stalebot believes this issue/PR has not been touched recently label Jul 25, 2019

Raul Gutierrez Segales added 3 commits July 28, 2019 10:18

Merge remote-tracking branch 'upstream/master' into zk-filter-parse-r…

b992d6b

…esponses

Add test coverage for responses

b91986b

This should cover most of it, I still need to test some corner cases. Signed-off-by: Raul Gutierrez Segales <rgs@pinterest.com>

Update counters documentation

6d07d1e

Signed-off-by: Raul Gutierrez Segales <rgs@pinterest.com>

rgs1 mentioned this pull request Jul 28, 2019

Fix RuntimeFromNonWorkerThreads #7746

Merged

Merge remote-tracking branch 'upstream/master' into zk-filter-parse-r…

b839e1c

…esponses

snowp suggested changes Jul 30, 2019

View reviewed changes

Address review

7ffa9cc

Signed-off-by: Raul Gutierrez Segales <rgs@pinterest.com>

snowp suggested changes Aug 1, 2019

View reviewed changes

source/extensions/filters/network/zookeeper_proxy/decoder.cc Outdated Show resolved Hide resolved

Simplify assert.

8f894ff

Signed-off-by: Raul Gutierrez Segales <rgs@pinterest.com>

snowp approved these changes Aug 2, 2019

View reviewed changes

snowp merged commit 266252e into envoyproxy:master Aug 2, 2019

Conversation

rgs1 commented Jul 14, 2019

Uh oh!

rgs1 commented Jul 14, 2019

Uh oh!

rgs1 commented Jul 14, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

rgs1 commented Jul 14, 2019

Uh oh!

repokitteh-read-only bot commented Jul 14, 2019

Uh oh!

stale bot commented Jul 25, 2019

Uh oh!

rgs1 commented Jul 25, 2019

Uh oh!

rgs1 commented Jul 28, 2019

Uh oh!

rgs1 commented Jul 28, 2019

Uh oh!

repokitteh-read-only bot commented Jul 28, 2019

Uh oh!

rgs1 commented Jul 28, 2019

Uh oh!

rgs1 commented Jul 28, 2019

Uh oh!

repokitteh-read-only bot commented Jul 28, 2019

Uh oh!

rgs1 commented Jul 28, 2019

Uh oh!

rgs1 commented Jul 28, 2019

Uh oh!

snowp left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

rgs1 commented Jul 31, 2019

Uh oh!

repokitteh-read-only bot commented Jul 31, 2019

Uh oh!

snowp left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

snowp left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

rgs1 commented Jul 14, 2019 •

edited

Loading