You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@skywalking.apache.org by "lujiajing1126 (via GitHub)" <gi...@apache.org> on 2023/10/03 07:59:14 UTC
[PR] Update BanyanDB Java Client to 0.5.0 [skywalking]
lujiajing1126 opened a new pull request, #11370:
URL: https://github.com/apache/skywalking/pull/11370
<!--
โ ๏ธ Please make sure to read this template first, pull requests that don't accord with this template
maybe closed without notice.
Texts surrounded by `<` and `>` are meant to be replaced by you, e.g. <framework name>, <issue number>.
Put an `x` in the `[ ]` to mark the item as CHECKED. `[x]`
-->
<!-- ==== ๐ Remove this line WHEN AND ONLY WHEN you're fixing a bug, follow the checklist ๐ ====
### Fix <bug description or the bug issue number or bug issue link>
- [ ] Add a unit test to verify that the fix works.
- [ ] Explain briefly why the bug exists and how to fix it.
==== ๐ Remove this line WHEN AND ONLY WHEN you're fixing a bug, follow the checklist ๐ ==== -->
<!-- ==== ๐ Remove this line WHEN AND ONLY WHEN you're improving the performance, follow the checklist ๐ ====
### Improve the performance of <class or module or ...>
- [ ] Add a benchmark for the improvement, refer to [the existing ones](https://github.com/apache/skywalking/blob/master/apm-commons/apm-datacarrier/src/test/java/org/apache/skywalking/apm/commons/datacarrier/LinkedArrayBenchmark.java)
- [ ] The benchmark result.
```text
<Paste the benchmark results here>
```
- [ ] Links/URLs to the theory proof or discussion articles/blogs. <links/URLs here>
==== ๐ Remove this line WHEN AND ONLY WHEN you're improving the performance, follow the checklist ๐ ==== -->
<!-- ==== ๐ Remove this line WHEN AND ONLY WHEN you're adding a new feature, follow the checklist ๐ ====
### <Feature description>
- [ ] If this is non-trivial feature, paste the links/URLs to the design doc.
- [ ] Update the documentation to include this new feature.
- [ ] Tests(including UT, IT, E2E) are added to verify the new feature.
- [ ] If it's UI related, attach the screenshots below.
==== ๐ Remove this line WHEN AND ONLY WHEN you're adding a new feature, follow the checklist ๐ ==== -->
- [ ] If this pull request closes/resolves/fixes an existing issue, replace the issue number. Closes #<issue number>.
- [x] Update the [`CHANGES` log](https://github.com/apache/skywalking/blob/master/docs/en/changes/changes.md).
--
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.
To unsubscribe, e-mail: notifications-unsubscribe@skywalking.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Update BanyanDB Java Client to 0.5.0 [skywalking]
Posted by "wu-sheng (via GitHub)" <gi...@apache.org>.
wu-sheng commented on PR #11370:
URL: https://github.com/apache/skywalking/pull/11370#issuecomment-1746165298
I have tried rerun, profiling e2e with BanyanDB fails. I am not sure whether it is relative.
--
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.
To unsubscribe, e-mail: notifications-unsubscribe@skywalking.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Update BanyanDB Java Client to 0.5.0 [skywalking]
Posted by "wu-sheng (via GitHub)" <gi...@apache.org>.
wu-sheng commented on code in PR #11370:
URL: https://github.com/apache/skywalking/pull/11370#discussion_r1345737175
##########
docs/en/changes/changes.md:
##########
@@ -13,6 +13,7 @@
including `ui_template๏ผui_menu๏ผcontinuous_profiling_policy`.
* Add a release mechanism for alarm windows when it is expired in case of OOM.
* Fix Zipkin trace receiver response: make the HTTP status code from `200` to `202`.
+* Update BanyanDB Java Client to 0.5.0.
Review Comment:
Please add a `fix` changelog 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.
To unsubscribe, e-mail: notifications-unsubscribe@skywalking.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Update BanyanDB Java Client to 0.5.0 [skywalking]
Posted by "wu-sheng (via GitHub)" <gi...@apache.org>.
wu-sheng commented on PR #11370:
URL: https://github.com/apache/skywalking/pull/11370#issuecomment-1745999680
@lujiajing1126 Please fix this https://github.com/apache/skywalking/pull/11370#discussion_r1344149146.
The Istio relative e2e are always failure due to GHA issues. I have setup the workflow to skip them.
--
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.
To unsubscribe, e-mail: notifications-unsubscribe@skywalking.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Update BanyanDB Java Client to 0.5.0 [skywalking]
Posted by "wu-sheng (via GitHub)" <gi...@apache.org>.
wu-sheng commented on code in PR #11370:
URL: https://github.com/apache/skywalking/pull/11370#discussion_r1345058964
##########
docs/en/setup/backend/storages/banyandb.md:
##########
@@ -6,8 +6,7 @@ Activate BanyanDB as the storage, and set storage provider to **banyandb**.
```yaml
storage:
banyandb:
- host: ${SW_STORAGE_BANYANDB_HOST:127.0.0.1}
- port: ${SW_STORAGE_BANYANDB_PORT:17912}
+ endpoints: ${SW_STORAGE_BANYANDB_ENDPOINTS:127.0.0.1:17912}
Review Comment:
Meanwhile, please bump the BanyanDB server version to the latest. We should close to 0.5 server release.
--
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.
To unsubscribe, e-mail: notifications-unsubscribe@skywalking.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Update BanyanDB Java Client to 0.5.0 [skywalking]
Posted by "lujiajing1126 (via GitHub)" <gi...@apache.org>.
lujiajing1126 commented on PR #11370:
URL: https://github.com/apache/skywalking/pull/11370#issuecomment-1746786810
> @lujiajing1126 We should have removed ID column, right? But in `BanyanDBMetadataQueryDAO`, `getInstances` doesn't work like `getInstance`. It still uses the ID as query condition, rather than decoding the ID and use series IDs.
>
> I think this is the root cause.
I see. Fixed.
--
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.
To unsubscribe, e-mail: notifications-unsubscribe@skywalking.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Update BanyanDB Java Client to 0.5.0 [skywalking]
Posted by "wu-sheng (via GitHub)" <gi...@apache.org>.
wu-sheng commented on PR #11370:
URL: https://github.com/apache/skywalking/pull/11370#issuecomment-1746067487
`configuration-vocabulary.md` should be updated too.
--
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.
To unsubscribe, e-mail: notifications-unsubscribe@skywalking.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Update BanyanDB Java Client to 0.5.0 [skywalking]
Posted by "wu-sheng (via GitHub)" <gi...@apache.org>.
wu-sheng commented on code in PR #11370:
URL: https://github.com/apache/skywalking/pull/11370#discussion_r1343744947
##########
docs/en/setup/backend/storages/banyandb.md:
##########
@@ -6,8 +6,7 @@ Activate BanyanDB as the storage, and set storage provider to **banyandb**.
```yaml
storage:
banyandb:
- host: ${SW_STORAGE_BANYANDB_HOST:127.0.0.1}
- port: ${SW_STORAGE_BANYANDB_PORT:17912}
+ endpoints: ${SW_STORAGE_BANYANDB_ENDPOINTS:127.0.0.1:17912}
Review Comment:
Let's use targets as client API says.
And with this change, e2e needs to change, as well as vocabulary docs. Because system env variable name changes.
--
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.
To unsubscribe, e-mail: notifications-unsubscribe@skywalking.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Update BanyanDB Java Client to 0.5.0 [skywalking]
Posted by "lujiajing1126 (via GitHub)" <gi...@apache.org>.
lujiajing1126 commented on code in PR #11370:
URL: https://github.com/apache/skywalking/pull/11370#discussion_r1343691385
##########
oap-server/server-storage-plugin/storage-banyandb-plugin/src/main/java/org/apache/skywalking/oap/server/storage/plugin/banyandb/BanyanDBStorageClient.java:
##########
@@ -56,7 +56,7 @@ public class BanyanDBStorageClient implements Client, HealthCheckable {
private final DelegatedHealthChecker healthChecker = new DelegatedHealthChecker();
public BanyanDBStorageClient(String host, int port) {
- this.client = new BanyanDBClient(host, port);
+ this.client = new BanyanDBClient(host + ":" + port);
Review Comment:
Fixed. Now a comma-separated string is supported
--
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.
To unsubscribe, e-mail: notifications-unsubscribe@skywalking.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Update BanyanDB Java Client to 0.5.0 [skywalking]
Posted by "wu-sheng (via GitHub)" <gi...@apache.org>.
wu-sheng commented on code in PR #11370:
URL: https://github.com/apache/skywalking/pull/11370#discussion_r1343672490
##########
oap-server/server-storage-plugin/storage-banyandb-plugin/src/main/java/org/apache/skywalking/oap/server/storage/plugin/banyandb/BanyanDBStorageClient.java:
##########
@@ -56,7 +56,7 @@ public class BanyanDBStorageClient implements Client, HealthCheckable {
private final DelegatedHealthChecker healthChecker = new DelegatedHealthChecker();
public BanyanDBStorageClient(String host, int port) {
- this.client = new BanyanDBClient(host, port);
+ this.client = new BanyanDBClient(host + ":" + port);
Review Comment:
We should change storage configurations with docs to accept multiple IP:port(s) for cluster server.
--
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.
To unsubscribe, e-mail: notifications-unsubscribe@skywalking.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Update BanyanDB Java Client to 0.5.0 [skywalking]
Posted by "lujiajing1126 (via GitHub)" <gi...@apache.org>.
lujiajing1126 commented on PR #11370:
URL: https://github.com/apache/skywalking/pull/11370#issuecomment-1746562082
> @lujiajing1126 It still fails, it seems server side has some issues.
```
Caused by: io.grpc.StatusRuntimeException: UNKNOWN: fail to analyze the query request for measure instance_traffic_minute: mandatory index rule conf:name:"id" op:BINARY_OP_IN value:{str_array:{value:"c3FydA==.1_dGVzdC1pbnN0YW5jZQ=="}}: unsupported condition operation: invalid query message
at io.grpc.stub.ClientCalls.toStatusRuntimeException(ClientCalls.java:271) ~[grpc-stub-1.53.0.jar:1.53.0]
at io.grpc.stub.ClientCalls.getUnchecked(ClientCalls.java:252) ~[grpc-stub-1.53.0.jar:1.53.0]
at io.grpc.stub.ClientCalls.blockingUnaryCall(ClientCalls.java:165) ~[grpc-stub-1.53.0.jar:1.53.0]
at org.apache.skywalking.banyandb.measure.v1.MeasureServiceGrpc$MeasureServiceBlockingStub.query(MeasureServiceGrpc.java:263) ~[banyandb-java-client-0.5.0.jar:0.5.0]
at org.apache.skywalking.banyandb.v1.client.BanyanDBClient.lambda$query$4(BanyanDBClient.java:386) ~[banyandb-java-client-0.5.0.jar:0.5.0]
at org.apache.skywalking.banyandb.v1.client.grpc.HandleExceptionsWith.callAndTranslateApiException(HandleExceptionsWith.java:45) ~[banyandb-java-client-0.5.0.jar:0.5.0]
at org.apache.skywalking.banyandb.v1.client.BanyanDBClient.query(BanyanDBClient.java:383) ~[banyandb-java-client-0.5.0.jar:0.5.0]
at org.apache.skywalking.oap.server.storage.plugin.banyandb.BanyanDBStorageClient.query(BanyanDBStorageClient.java:128) ~[storage-banyandb-plugin-9.7.0-SNAPSHOT.jar:9.7.0-SNAPSHOT]
at org.apache.skywalking.oap.server.storage.plugin.banyandb.stream.AbstractBanyanDBDAO.query(AbstractBanyanDBDAO.java:129) ~[storage-banyandb-plugin-9.7.0-SNAPSHOT.jar:9.7.0-SNAPSHOT]
at org.apache.skywalking.oap.server.storage.plugin.banyandb.stream.AbstractBanyanDBDAO.query(AbstractBanyanDBDAO.java:115) ~[storage-banyandb-plugin-9.7.0-SNAPSHOT.jar:9.7.0-SNAPSHOT]
at org.apache.skywalking.oap.server.storage.plugin.banyandb.stream.AbstractBanyanDBDAO.query(AbstractBanyanDBDAO.java:80) ~[storage-banyandb-plugin-9.7.0-SNAPSHOT.jar:9.7.0-SNAPSHOT]
at org.apache.skywalking.oap.server.storage.plugin.banyandb.measure.BanyanDBMetadataQueryDAO.getInstances(BanyanDBMetadataQueryDAO.java:157) ~[storage-banyandb-plugin-9.7.0-SNAPSHOT.jar:9.7.0-SNAPSHOT]
at org.apache.skywalking.oap.server.core.profiling.continuous.ContinuousProfilingQueryService.queryContinuousProfilingMonitoringInstances(ContinuousProfilingQueryService.java:150) ~[server-core-9.7.0-SNAPSHOT.jar:9.7.0-SNAPSHOT]
at org.apache.skywalking.oap.query.graphql.resolver.ContinuousProfilingQuery.queryContinuousProfilingMonitoringInstances(ContinuousProfilingQuery.java:54) ~[query-graphql-plugin-9.7.0-SNAPSHOT.jar:9.7.0-SNAPSHOT]
at jdk.internal.reflect.GeneratedMethodAccessor130.invoke(Unknown Source) ~[?:?]
at jdk.internal.reflect.DelegatingMethodAccessorImpl.invoke(Unknown Source) ~[?:?]
at java.lang.reflect.Method.invoke(Unknown Source) ~[?:?]
at graphql.kickstart.tools.resolver.MethodFieldResolverDataFetcher.get(MethodFieldResolver.kt:282) ~[graphql-java-tools-13.0.1.jar:?]
... 44 more
```
The problem is `id` index is missing for `instance_traffic_minute`. Any idea why this could work in v0.4.0? @hanahmily
--
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.
To unsubscribe, e-mail: notifications-unsubscribe@skywalking.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Update BanyanDB Java Client to 0.5.0 [skywalking]
Posted by "wu-sheng (via GitHub)" <gi...@apache.org>.
wu-sheng commented on PR #11370:
URL: https://github.com/apache/skywalking/pull/11370#issuecomment-1744414894
FYI @wankai123
--
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.
To unsubscribe, e-mail: notifications-unsubscribe@skywalking.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Update BanyanDB Java Client to 0.5.0 [skywalking]
Posted by "wu-sheng (via GitHub)" <gi...@apache.org>.
wu-sheng commented on PR #11370:
URL: https://github.com/apache/skywalking/pull/11370#issuecomment-1746706644
@lujiajing1126 We should have removed ID column, right? But in `BanyanDBMetadataQueryDAO`, `getInstances` doesn't work like `getInstance`. It still uses the ID as query condition, rather than decoding the ID and use series IDs.
I think this is the root cause.
--
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.
To unsubscribe, e-mail: notifications-unsubscribe@skywalking.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Update BanyanDB Java Client to 0.5.0 [skywalking]
Posted by "wu-sheng (via GitHub)" <gi...@apache.org>.
wu-sheng merged PR #11370:
URL: https://github.com/apache/skywalking/pull/11370
--
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.
To unsubscribe, e-mail: notifications-unsubscribe@skywalking.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Update BanyanDB Java Client to 0.5.0 [skywalking]
Posted by "wu-sheng (via GitHub)" <gi...@apache.org>.
wu-sheng commented on PR #11370:
URL: https://github.com/apache/skywalking/pull/11370#issuecomment-1746192598
@lujiajing1126 It still fails, it seems server side has some issues.
--
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.
To unsubscribe, e-mail: notifications-unsubscribe@skywalking.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Update BanyanDB Java Client to 0.5.0 [skywalking]
Posted by "lujiajing1126 (via GitHub)" <gi...@apache.org>.
lujiajing1126 commented on code in PR #11370:
URL: https://github.com/apache/skywalking/pull/11370#discussion_r1343691385
##########
oap-server/server-storage-plugin/storage-banyandb-plugin/src/main/java/org/apache/skywalking/oap/server/storage/plugin/banyandb/BanyanDBStorageClient.java:
##########
@@ -56,7 +56,7 @@ public class BanyanDBStorageClient implements Client, HealthCheckable {
private final DelegatedHealthChecker healthChecker = new DelegatedHealthChecker();
public BanyanDBStorageClient(String host, int port) {
- this.client = new BanyanDBClient(host, port);
+ this.client = new BanyanDBClient(host + ":" + port);
Review Comment:
Fixed. Now a comma-separate string is supported
--
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.
To unsubscribe, e-mail: notifications-unsubscribe@skywalking.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Update BanyanDB Java Client to 0.5.0 [skywalking]
Posted by "wu-sheng (via GitHub)" <gi...@apache.org>.
wu-sheng commented on PR #11370:
URL: https://github.com/apache/skywalking/pull/11370#issuecomment-1746574948
@hanahmily told me its an OAP banyandb storage plugin bug. He will comment soon.
Let's see.
--
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.
To unsubscribe, e-mail: notifications-unsubscribe@skywalking.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Update BanyanDB Java Client to 0.5.0 [skywalking]
Posted by "wu-sheng (via GitHub)" <gi...@apache.org>.
wu-sheng commented on code in PR #11370:
URL: https://github.com/apache/skywalking/pull/11370#discussion_r1344149146
##########
docs/en/setup/backend/storages/banyandb.md:
##########
@@ -6,8 +6,7 @@ Activate BanyanDB as the storage, and set storage provider to **banyandb**.
```yaml
storage:
banyandb:
- host: ${SW_STORAGE_BANYANDB_HOST:127.0.0.1}
- port: ${SW_STORAGE_BANYANDB_PORT:17912}
+ endpoints: ${SW_STORAGE_BANYANDB_ENDPOINTS:127.0.0.1:17912}
Review Comment:
```suggestion
targets: ${SW_STORAGE_BANYANDB_TARGETS:127.0.0.1:17912}
```
Ref from there, https://github.com/apache/skywalking-banyandb-java-client/blob/4872fd2b61400e0602960c36ed0406672b2a5852/src/main/java/org/apache/skywalking/banyandb/v1/client/BanyanDBClient.java#L138
--
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.
To unsubscribe, e-mail: notifications-unsubscribe@skywalking.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org