You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@skywalking.apache.org by GitBox <gi...@apache.org> on 2022/04/27 01:41:13 UTC
[GitHub] [skywalking-banyandb-java-client] lujiajing1126 opened a new pull request, #11: Truncate duration unit at days
lujiajing1126 opened a new pull request, #11:
URL: https://github.com/apache/skywalking-banyandb-java-client/pull/11
This PR tries to resolve the following issues,
1. truncate duration unit at days (Fix https://github.com/apache/skywalking/issues/8948)
2. expose methods to developers,
3. update metadata cache on `get` (issue found by @dmsolr)
--
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
[GitHub] [skywalking-banyandb-java-client] wu-sheng commented on a diff in pull request #11: Truncate duration unit at days
Posted by GitBox <gi...@apache.org>.
wu-sheng commented on code in PR #11:
URL: https://github.com/apache/skywalking-banyandb-java-client/pull/11#discussion_r859813989
##########
src/main/java/org/apache/skywalking/banyandb/v1/client/BanyanDBClient.java:
##########
@@ -373,7 +386,14 @@ private void defineIndexRules(Stream stream, List<IndexRule> indexRules) throws
IndexRuleMetadataRegistry irRegistry = new IndexRuleMetadataRegistry(checkNotNull(this.channel));
for (final IndexRule ir : indexRules) {
- irRegistry.create(ir);
+ try {
+ irRegistry.create(ir);
+ } catch (BanyanDBException ex) {
+ if (ex.getStatus().equals(Status.Code.ALREADY_EXISTS)) {
+ continue;
+ }
Review Comment:
If we have `getxxx`, we should take 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.
To unsubscribe, e-mail: notifications-unsubscribe@skywalking.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [skywalking-banyandb-java-client] hanahmily commented on pull request #11: Truncate duration unit at days
Posted by GitBox <gi...@apache.org>.
hanahmily commented on PR #11:
URL: https://github.com/apache/skywalking-banyandb-java-client/pull/11#issuecomment-1111598155
I submitted https://github.com/apache/skywalking/issues/8965 to track this.
On Wed, Apr 27, 2022 at 11:17 PM Jiajing LU ***@***.***>
wrote:
> ***@***.**** commented on this pull request.
> ------------------------------
>
> In
> src/main/java/org/apache/skywalking/banyandb/v1/client/BanyanDBClient.java
> <https://github.com/apache/skywalking-banyandb-java-client/pull/11#discussion_r859929047>
> :
>
> > + irRegistry.create(ir);
> + } catch (BanyanDBException ex) {
> + if (ex.getStatus().equals(Status.Code.ALREADY_EXISTS)) {
> + continue;
> + }
>
> We have a series of getXXX to check this.
>
> BTW, we can't avoid this exception under a concurrent creating scenario
> even get the resource before creating it. I tend to leave this piece as it
> is and add a getting operation before the creating.
>
> IIRC getXXX will throw NOT_FOUND now if no schema is found...We have to
> check that exception too...
>
> —
> Reply to this email directly, view it on GitHub
> <https://github.com/apache/skywalking-banyandb-java-client/pull/11#discussion_r859929047>,
> or unsubscribe
> <https://github.com/notifications/unsubscribe-auth/AAO6UWONKCT4OQWJIZPN4ILVHFK7XANCNFSM5UNSOVCA>
> .
> You are receiving this because you were mentioned.Message ID:
> ***@***.***
> com>
>
--
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
[GitHub] [skywalking-banyandb-java-client] lujiajing1126 commented on a diff in pull request #11: Truncate duration unit at days
Posted by GitBox <gi...@apache.org>.
lujiajing1126 commented on code in PR #11:
URL: https://github.com/apache/skywalking-banyandb-java-client/pull/11#discussion_r859814384
##########
src/main/java/org/apache/skywalking/banyandb/v1/client/BanyanDBClient.java:
##########
@@ -411,6 +439,26 @@ private void defineIndexRules(Measure measure, List<IndexRule> indexRules) throw
irbRegistry.create(binding);
}
+ /**
+ * Try to find the group defined
+ *
+ * @param name name of the group
+ * @return the group found in BanyanDB
+ */
+ public Group findGroup(String name) throws BanyanDBException {
Review Comment:
Yes. We could do this.
--
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
[GitHub] [skywalking-banyandb-java-client] wu-sheng merged pull request #11: Truncate duration unit at days
Posted by GitBox <gi...@apache.org>.
wu-sheng merged PR #11:
URL: https://github.com/apache/skywalking-banyandb-java-client/pull/11
--
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
[GitHub] [skywalking-banyandb-java-client] lujiajing1126 commented on a diff in pull request #11: Truncate duration unit at days
Posted by GitBox <gi...@apache.org>.
lujiajing1126 commented on code in PR #11:
URL: https://github.com/apache/skywalking-banyandb-java-client/pull/11#discussion_r859785654
##########
src/main/java/org/apache/skywalking/banyandb/v1/client/BanyanDBClient.java:
##########
@@ -373,7 +386,14 @@ private void defineIndexRules(Stream stream, List<IndexRule> indexRules) throws
IndexRuleMetadataRegistry irRegistry = new IndexRuleMetadataRegistry(checkNotNull(this.channel));
for (final IndexRule ir : indexRules) {
- irRegistry.create(ir);
+ try {
+ irRegistry.create(ir);
+ } catch (BanyanDBException ex) {
+ if (ex.getStatus().equals(Status.Code.ALREADY_EXISTS)) {
+ continue;
+ }
Review Comment:
> @hanahmily Could you track this? We should add one, this is not a good practice.
If we add `isExist`, is it possible we would introduce a potential data race?
--
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
[GitHub] [skywalking-banyandb-java-client] lujiajing1126 commented on a diff in pull request #11: Truncate duration unit at days
Posted by GitBox <gi...@apache.org>.
lujiajing1126 commented on code in PR #11:
URL: https://github.com/apache/skywalking-banyandb-java-client/pull/11#discussion_r859774001
##########
src/main/java/org/apache/skywalking/banyandb/v1/client/BanyanDBClient.java:
##########
@@ -373,7 +386,14 @@ private void defineIndexRules(Stream stream, List<IndexRule> indexRules) throws
IndexRuleMetadataRegistry irRegistry = new IndexRuleMetadataRegistry(checkNotNull(this.channel));
for (final IndexRule ir : indexRules) {
- irRegistry.create(ir);
+ try {
+ irRegistry.create(ir);
+ } catch (BanyanDBException ex) {
+ if (ex.getStatus().equals(Status.Code.ALREADY_EXISTS)) {
+ continue;
+ }
Review Comment:
> In code practice, we should not use `exception` to control workflow. Please use `isExist` before `create`.
Currently, we don't have `isExist` API.
--
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
[GitHub] [skywalking-banyandb-java-client] wu-sheng commented on a diff in pull request #11: Truncate duration unit at days
Posted by GitBox <gi...@apache.org>.
wu-sheng commented on code in PR #11:
URL: https://github.com/apache/skywalking-banyandb-java-client/pull/11#discussion_r859792298
##########
src/main/java/org/apache/skywalking/banyandb/v1/client/BanyanDBClient.java:
##########
@@ -373,7 +386,14 @@ private void defineIndexRules(Stream stream, List<IndexRule> indexRules) throws
IndexRuleMetadataRegistry irRegistry = new IndexRuleMetadataRegistry(checkNotNull(this.channel));
for (final IndexRule ir : indexRules) {
- irRegistry.create(ir);
+ try {
+ irRegistry.create(ir);
+ } catch (BanyanDBException ex) {
+ if (ex.getStatus().equals(Status.Code.ALREADY_EXISTS)) {
+ continue;
+ }
Review Comment:
The current logic should be kept as it could reduce 99% exception cases, the lock mechanism has to exist due to avoid race and inconsistent data.
--
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
[GitHub] [skywalking-banyandb-java-client] hanahmily commented on a diff in pull request #11: Truncate duration unit at days
Posted by GitBox <gi...@apache.org>.
hanahmily commented on code in PR #11:
URL: https://github.com/apache/skywalking-banyandb-java-client/pull/11#discussion_r859807876
##########
src/main/java/org/apache/skywalking/banyandb/v1/client/BanyanDBClient.java:
##########
@@ -373,7 +386,14 @@ private void defineIndexRules(Stream stream, List<IndexRule> indexRules) throws
IndexRuleMetadataRegistry irRegistry = new IndexRuleMetadataRegistry(checkNotNull(this.channel));
for (final IndexRule ir : indexRules) {
- irRegistry.create(ir);
+ try {
+ irRegistry.create(ir);
+ } catch (BanyanDBException ex) {
+ if (ex.getStatus().equals(Status.Code.ALREADY_EXISTS)) {
+ continue;
+ }
Review Comment:
We have a series of `getXXX` to check this.
BTW, we can't avoid this exception under a concurrent creating scenario even get the resource before creating it. I tend to leave this piece as it is and add a getting operation before the creating.
--
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
[GitHub] [skywalking-banyandb-java-client] wu-sheng commented on a diff in pull request #11: Truncate duration unit at days
Posted by GitBox <gi...@apache.org>.
wu-sheng commented on code in PR #11:
URL: https://github.com/apache/skywalking-banyandb-java-client/pull/11#discussion_r859779013
##########
src/main/java/org/apache/skywalking/banyandb/v1/client/BanyanDBClient.java:
##########
@@ -373,7 +386,14 @@ private void defineIndexRules(Stream stream, List<IndexRule> indexRules) throws
IndexRuleMetadataRegistry irRegistry = new IndexRuleMetadataRegistry(checkNotNull(this.channel));
for (final IndexRule ir : indexRules) {
- irRegistry.create(ir);
+ try {
+ irRegistry.create(ir);
+ } catch (BanyanDBException ex) {
+ if (ex.getStatus().equals(Status.Code.ALREADY_EXISTS)) {
+ continue;
+ }
Review Comment:
@hanahmily Could you track this? We should add one, this is not a good practice.
--
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
[GitHub] [skywalking-banyandb-java-client] lujiajing1126 commented on a diff in pull request #11: Truncate duration unit at days
Posted by GitBox <gi...@apache.org>.
lujiajing1126 commented on code in PR #11:
URL: https://github.com/apache/skywalking-banyandb-java-client/pull/11#discussion_r859929047
##########
src/main/java/org/apache/skywalking/banyandb/v1/client/BanyanDBClient.java:
##########
@@ -373,7 +386,14 @@ private void defineIndexRules(Stream stream, List<IndexRule> indexRules) throws
IndexRuleMetadataRegistry irRegistry = new IndexRuleMetadataRegistry(checkNotNull(this.channel));
for (final IndexRule ir : indexRules) {
- irRegistry.create(ir);
+ try {
+ irRegistry.create(ir);
+ } catch (BanyanDBException ex) {
+ if (ex.getStatus().equals(Status.Code.ALREADY_EXISTS)) {
+ continue;
+ }
Review Comment:
> We have a series of `getXXX` to check this.
>
> BTW, we can't avoid this exception under a concurrent creating scenario even get the resource before creating it. I tend to leave this piece as it is and add a getting operation before the creating.
IIRC `getXXX` will throw `NOT_FOUND` now if no schema is found...We have to check that exception 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
[GitHub] [skywalking-banyandb-java-client] wu-sheng commented on a diff in pull request #11: Truncate duration unit at days
Posted by GitBox <gi...@apache.org>.
wu-sheng commented on code in PR #11:
URL: https://github.com/apache/skywalking-banyandb-java-client/pull/11#discussion_r859810536
##########
src/main/java/org/apache/skywalking/banyandb/v1/client/BanyanDBClient.java:
##########
@@ -411,6 +439,26 @@ private void defineIndexRules(Measure measure, List<IndexRule> indexRules) throw
irbRegistry.create(binding);
}
+ /**
+ * Try to find the group defined
+ *
+ * @param name name of the group
+ * @return the group found in BanyanDB
+ */
+ public Group findGroup(String name) throws BanyanDBException {
Review Comment:
```suggestion
public Optional<Group> findGroup(String name) throws BanyanDBException {
```
Consider using Optional if this is expected to nullable. We are using JDK8+, right?
--
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
[GitHub] [skywalking-banyandb-java-client] wu-sheng commented on a diff in pull request #11: Truncate duration unit at days
Posted by GitBox <gi...@apache.org>.
wu-sheng commented on code in PR #11:
URL: https://github.com/apache/skywalking-banyandb-java-client/pull/11#discussion_r859464743
##########
src/main/java/org/apache/skywalking/banyandb/v1/client/BanyanDBClient.java:
##########
@@ -373,7 +386,14 @@ private void defineIndexRules(Stream stream, List<IndexRule> indexRules) throws
IndexRuleMetadataRegistry irRegistry = new IndexRuleMetadataRegistry(checkNotNull(this.channel));
for (final IndexRule ir : indexRules) {
- irRegistry.create(ir);
+ try {
+ irRegistry.create(ir);
+ } catch (BanyanDBException ex) {
+ if (ex.getStatus().equals(Status.Code.ALREADY_EXISTS)) {
+ continue;
+ }
Review Comment:
In code practice, we should not use `exception` to control workflow. Please use `isExist` before `create`.
--
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