[GitHub] [apisix] Yiyiyimu commented on pull request #2036: feature: support etcd v3, by mocking v2 API
Yiyiyimu commented on pull request #2036: URL: https://github.com/apache/apisix/pull/2036#issuecomment-693536211 TODO: we need to add doc for etcd migration This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [apisix] Yiyiyimu commented on pull request #2036: feature: support etcd v3, by mocking v2 API
Yiyiyimu commented on pull request #2036: URL: https://github.com/apache/apisix/pull/2036#issuecomment-693426643 > we could optimize it in another pr @nic-chen I think we need to release a newer version of lua-resty-etcd, since the current version would output TONS of logs. This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [apisix] Yiyiyimu commented on pull request #2036: feature: support etcd v3, by mocking v2 API
Yiyiyimu commented on pull request #2036:
URL: https://github.com/apache/apisix/pull/2036#issuecomment-69271
Currently the log produced by lua-resty-etcd would show message without
base64 decode. For example
```
2020/09/15 23:18:52 [info] 1964#1964: *2 [lua] v3.lua:284: set(): v3 set
body:
{"prev_kv":{"value":"InRlc3RfdmFsdWUi","create_revision":"149","mod_revision":"150","key":"L2FwaXNpeC90ZXN0X2tleQ==","version":"2"},"header":{"raft_term":"2","cluster_id":"8925027824743593106","member_id":"13803658152347727308","revision":"151"}},
client: 127.0.0.1, server: localhost, request: "GET /t HTTP/1.1", host:
"localhost"
2020/09/15 23:18:52 [info] 1964#1964: *15 [lua] v3.lua:500: request_chunk():
http request method: POST path: /v3/watch body:
{"create_request":{"range_end":"L2FwaXNpeC91cHN0cmVhbXQ=","start_revision":151,"key":"L2FwaXNpeC91cHN0cmVhbXM="}}
query: nil, context: ngx.timer
```
Shall we do the decode on etcd side, or just remove these logs like v2 did?
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
[GitHub] [apisix] Yiyiyimu commented on pull request #2036: feature: support etcd v3, by mocking v2 API
Yiyiyimu commented on pull request #2036: URL: https://github.com/apache/apisix/pull/2036#issuecomment-692553222 > > one more thing, we need to check the `etcd` version in `bin/apisix`, confirm the `etcd` version `>= 3.4` . > > we can fix this in a new PR, here is the related issue: #2227 fix #2227 in new commit This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [apisix] Yiyiyimu commented on pull request #2036: feature: support etcd v3, by mocking v2 API
Yiyiyimu commented on pull request #2036: URL: https://github.com/apache/apisix/pull/2036#issuecomment-689952623 The newly imported lib [base64](https://github.com/iskolbin/lbase64) holds MIT License and only 200 lines. So I think we could directly use it. This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [apisix] Yiyiyimu commented on pull request #2036: feature: support etcd v3, by mocking v2 API
Yiyiyimu commented on pull request #2036: URL: https://github.com/apache/apisix/pull/2036#issuecomment-688697918 > I added this PR to 2.0 milestone, which due by September 18, 2020 Sure. It just I need some help in the current problem, which discussed in https://github.com/apache/apisix/pull/2036#discussion_r483905871 This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [apisix] Yiyiyimu commented on pull request #2036: feature: support etcd v3, by mocking v2 API
Yiyiyimu commented on pull request #2036: URL: https://github.com/apache/apisix/pull/2036#issuecomment-686846176 Hi @membphis I noticed there are still errrors in your latest commit like https://github.com/apache/apisix/runs/1046997001#step:6:423, but since there is no `no_error_log` add to the unit test, it's still be regarded as success in CI. Is there any reason that we do not add `no_error_log` to these or we do need to do so. This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [apisix] Yiyiyimu commented on pull request #2036: feature: support etcd v3, by mocking v2 API
Yiyiyimu commented on pull request #2036: URL: https://github.com/apache/apisix/pull/2036#issuecomment-680966931 @membphis it seems I could not reply to your comment on "etcd v2 version". The original implementation should directly use the version detection in lua-resty-etcd, but it is somehow not working, as I stated here: https://github.com/apache/apisix/pull/2036#discussion_r475052806 This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [apisix] Yiyiyimu commented on pull request #2036: feature: support etcd v3, by mocking v2 API
Yiyiyimu commented on pull request #2036: URL: https://github.com/apache/apisix/pull/2036#issuecomment-680923213 > No, apisix and etcd will not in the same machine. If they are not on the same machine, could etcdctl installed on apisix's machine connect to the other machine's etcd? If not and the only way to connect the other machine's etcd is through gRPC gateway, I think I'll some more time on this feature. This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [apisix] Yiyiyimu commented on pull request #2036: feature: support etcd v3, by mocking v2 API
Yiyiyimu commented on pull request #2036: URL: https://github.com/apache/apisix/pull/2036#issuecomment-678654583 > I think we can merge this PR at next week. That would be great! One more question, which place in doc do you suggest to put the etcd migration link? Like we could put it in the main README.md temporarily to remind users and maybe we need someplace else permanently. This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [apisix] Yiyiyimu commented on pull request #2036: feature: support etcd v3, by mocking v2 API
Yiyiyimu commented on pull request #2036: URL: https://github.com/apache/apisix/pull/2036#issuecomment-674549680 > Use v3 or v2 protocol, their test results should be the same. Because `etcd` is incrementally notified, most requests are in the `wait` status. Is there any reason that could cause the difference? Or the default benchmark test is not suitable for this change This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [apisix] Yiyiyimu commented on pull request #2036: feature: support etcd v3, by mocking v2 API
Yiyiyimu commented on pull request #2036: URL: https://github.com/apache/apisix/pull/2036#issuecomment-672564838 **DEBUG HELP NEEDED!!** Currently there is only one error in test file, which is the last test of t/plugin/key-auth.t. Normally in etcd v2, it would add 20 consumers and find the 13th. But in current implementation of etcd v3, the test would add 20 consumers but only get first three of them, so it could not get the 13th. However, if I rerun the test, the test would get all 20 consumers and passed. I think it might related to the implementation of waitdir between two versions, but I could not find the way to debug. I print `self.values_hash` at the start of each `sync_data`, and the log is [here](https://gist.github.com/Yiyiyimu/f98647847ef49440454c9fff8a4f7295), maybe it could be of help. This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [apisix] Yiyiyimu commented on pull request #2036: feature: support etcd v3, by mocking v2 API
Yiyiyimu commented on pull request #2036: URL: https://github.com/apache/apisix/pull/2036#issuecomment-671945683 Currently implemented multi-version support (auto change etcd prefix) in t/APISIX.pm, worked for most tests. But for test files like t/admin/stream-routes-disable.t, which change the content of config.yaml before test would not get the prefix change in APISIX.pm. So right now mannually set etcd prefix to "/v3alpha" to try to pass the github ci This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [apisix] Yiyiyimu commented on pull request #2036: feature: support etcd v3, by mocking v2 API
Yiyiyimu commented on pull request #2036: URL: https://github.com/apache/apisix/pull/2036#issuecomment-671440970 It seems travis ci [passed in my personal repo](https://travis-ci.com/github/Yiyiyimu/incubator-apisix/builds/179139937), but github CI failed quite early. I'm a bit confused here. This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [apisix] Yiyiyimu commented on pull request #2036: feature: support etcd v3, by mocking v2 API
Yiyiyimu commented on pull request #2036: URL: https://github.com/apache/apisix/pull/2036#issuecomment-671412054 TODO: - [ ] currenly only test v3, **test for both v2 and v3** - [ ] currently it only support etcd v2 and v3.3+, **do we need to support other etcd version between these two**? - [ ] documentation of migrating data from v2 to v3, to avoid data loss This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [apisix] Yiyiyimu commented on pull request #2036: feature: support etcd v3, by mocking v2 API
Yiyiyimu commented on pull request #2036: URL: https://github.com/apache/apisix/pull/2036#issuecomment-671401256 I make a default benchmark test on my local PC, it seems that deploying etcd v3 could improve performance quite a lot || v3 | v2 | | -- | | | | 1 worker + 1 upstream + no plugin | + sleep 1+ wrk -d 5 -c 16 http://127.0.0.1:9080/helloRunning 5s test @ http://127.0.0.1:9080/hello 2 threads and 16 connections Thread Stats Avg Stdev Max +/- StdevLatency 1.17ms 153.33us 3.44ms 77.84%Req/Sec 6.83k 271.91 7.53k72.55% 69316 requests in 5.10s, 275.85MB readRequests/sec: 13591.19Transfer/sec: 54.09MB+ sleep 1+ wrk -d 5 -c 16 http://127.0.0.1:9080/helloRunning 5s test @ http://127.0.0.1:9080/hello 2 threads and 16 connections Thread Stats Avg Stdev Max +/- StdevLatency 1.18ms 176.65us 5.14ms 83.33%Req/Sec 6.80k 268.62 7.48k73.53% 68972 requests in 5.10s, 274.49MB readRequests/sec: 13524.55Transfer/sec: 53.82MB | + sleep 1+ wrk -d 5 -c 16 http://127.0.0.1:9080/helloRunning 5s test @ http://127.0.0.1:9080/hello 2 threads and 16 connections Thread Stats Avg Stdev Max +/- StdevLatency 1.33ms 211.08us 5.95ms 82.60%Req/Sec 6.05k 289.03 6.63k71.57% 61423 requests in 5.10s, 245.32MB readRequests/sec: 12043.51Transfer/sec: 48.10MB+ sleep 1+ wrk -d 5 -c 16 http://127.0.0.1:9080/helloRunning 5s test @ http://127.0.0.1:9080/hello 2 threads and 16 connections Thread Stats Avg Stdev Max +/- StdevLatency 1.31ms 195.78us 3.63ms 81.55%Req/Sec 6.11k 600.9411.41k97.03% 61396 requests in 5.10s, 245.22MB readRequests/sec: 12039.57Transfer/sec: 48.09MB | | 1 worker + 1 upstream + 2 plugins | + sleep 3+ wrk -d 5 -c 16 http://127.0.0.1:9080/helloRunning 5s test @ http://127.0.0.1:9080/hello 2 threads and 16 connections Thread Stats Avg Stdev Max +/- StdevLatency 558.87us1.24ms 35.81ms 96.89%Req/Sec19.64k 1.91k 28.10k76.24% 197213 requests in 5.10s, 801.24MB readRequests/sec: 38669.03Transfer/sec:157.11MB+ sleep 1+ wrk -d 5 -c 16 http://127.0.0.1:9080/helloRunning 5s test @ http://127.0.0.1:9080/hello 2 threads and 16 connections Thread Stats Avg Stdev Max +/- StdevLatency 521.08us1.05ms 25.48ms 97.08%Req/Sec19.72k 1.84k 23.12k73.00% 196309 requests in 5.00s, 797.58MB readRequests/sec: 39251.72Transfer/sec:159.47MB | + sleep 3+ wrk -d 5 -c 16 http://127.0.0.1:9080/helloRunning 5s test @ http://127.0.0.1:9080/hello 2 threa ds and 16 connections Thread Stats Avg Stdev Max +/- StdevLatency 1.47ms 252.12us 9.54ms 84.44%Req/Sec 5.48k 588.3710.98k97.03% 55055 requests in 5.10s, 223.67MB readRequests/sec: 10796.49Transfer/sec: 43.86MB+ sleep 1+ wrk -d 5 -c 16 http://127.0.0.1:9080/helloRunning 5s test @ http://127.0.0.1:9080/hello 2 threads and 16 connections Thread Stats Avg Stdev Max +/- StdevLatency 1.51ms 292.56us 9.64ms 88.99%Req/Sec 5.34k 540.9610.24k97.03% 53658 requests in 5.10s, 217.99MB readRequests/sec: 10521.64Transfer/sec: 42.75MB | | fake empty apisix server: 1 worker | + sleep 1+ wrk -d 5 -c 16 http://127.0.0.1:9080/helloRunning 5s test @ http://127.0.0.1:9080/hello 2 threads and 16 connections Thread Stats Avg Stdev Max +/- StdevLatency 1.20ms 249.70us 8.56ms 90.65%Req/Sec 6.71k 299.13 7.87k75.49% 68112 requests in 5.10s, 271.06MB readRequests/sec: 13356.74Transfer/sec: 53.15MB+ sleep 1+ wrk -d 5 -c 16 http://127.0.0.1:9080/helloRunning 5s test @ http://127.0.0.1:9080/hello 2 threads and 16 connections Thread Stats Avg Stdev Max +/- StdevLatency 1.27ms1.00ms 18.01ms 98.38%Req/Sec 6.74k 786.8513.37k96.04% 67753 requests in 5.10s, 269.64MB readRequests/sec: 13287.07Transfer/sec: 52.88MB | + sleep 1+ wrk -d 5 -c 16 http://127.0.0.1:9080/helloRunning 5s test @ http://127.0.0.1:9080/hello 2 threads and 16 connections Thread Stats Avg Stdev Max +/- StdevLatency 1.17ms 153.33us 3.44ms 77.84%Req/Sec 6.83k 271.91 7.53k72.55% 69316 requests in 5.10s, 275.85MB readRequests/sec: 13591.19Transfer/sec: 54.09MB+ sleep 1+ wrk -d 5 -c 16 http://127.0.0.1:9080/helloRunning 5s test @ http://127.0.0.1:9080/hello 2 threads and 16 connections Thread Stats Avg Stdev
