You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@ignite.apache.org by GitBox <gi...@apache.org> on 2021/04/29 13:38:39 UTC
[GitHub] [ignite-3] kgusakov opened a new pull request #113: IGNITE-14670 Update metastorage manager with: range with applied revision, invoke for multiple updates.
kgusakov opened a new pull request #113:
URL: https://github.com/apache/ignite-3/pull/113
--
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:
users@infra.apache.org
[GitHub] [ignite-3] alievmirza commented on a change in pull request #113: IGNITE-14670 Update metastorage manager with: range with applied revision, invoke for multiple updates.
Posted by GitBox <gi...@apache.org>.
alievmirza commented on a change in pull request #113:
URL: https://github.com/apache/ignite-3/pull/113#discussion_r623071151
##########
File path: modules/metastorage-client/src/main/java/org/apache/ignite/metastorage/client/MetaStorageService.java
##########
@@ -215,8 +214,8 @@
*/
// TODO: https://issues.apache.org/jira/browse/IGNITE-14269: will be replaced by conditional multi update.
@NotNull
- CompletableFuture<Boolean> invoke(@NotNull Key key, @NotNull Condition condition,
- @NotNull Operation success, @NotNull Operation failure);
+ CompletableFuture<Boolean> invoke(@NotNull Condition condition,
Review comment:
Seems that we need to change javadoc as well, we're using collections now
##########
File path: modules/metastorage/src/main/java/org/apache/ignite/internal/metastorage/MetaStorageManager.java
##########
@@ -333,6 +342,27 @@ public synchronized void deployWatches() {
return metaStorageSvc.range(keyFrom, keyTo, revUpperBound);
}
+ /**
+ * Retrieves entries for the given key range in lexicographic order.
+ * Entries will be filtered out by the current applied revision as an upper bound
Review comment:
Probably we should explain here what applied revision is.
##########
File path: modules/metastorage/src/main/java/org/apache/ignite/internal/metastorage/MetaStorageManager.java
##########
@@ -333,6 +342,27 @@ public synchronized void deployWatches() {
return metaStorageSvc.range(keyFrom, keyTo, revUpperBound);
}
+ /**
+ * Retrieves entries for the given key range in lexicographic order.
+ * Entries will be filtered out by the current applied revision as an upper bound
+ *
+ * @param keyFrom Start key of range (inclusive). Couldn't be {@code null}.
+ * @param keyTo End key of range (exclusive). Could be {@code null}.
+ * @return Cursor built upon entries corresponding to the given range and revision.
Review comment:
I would change javadoc for @ return, there is no given revision, appliedRevision is used
--
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:
users@infra.apache.org
[GitHub] [ignite-3] sk0x50 commented on a change in pull request #113: IGNITE-14670 Update metastorage manager with: range with applied revision, invoke for multiple updates.
Posted by GitBox <gi...@apache.org>.
sk0x50 commented on a change in pull request #113:
URL: https://github.com/apache/ignite-3/pull/113#discussion_r625701920
##########
File path: modules/metastorage-common/src/main/java/org/apache/ignite/metastorage/common/Condition.java
##########
@@ -64,11 +64,16 @@ public boolean test(Entry e) {
*/
private long rev;
+ /**
+ * Key of entry, which will be tested for condition.
Review comment:
It should be a one-line comment.
##########
File path: modules/metastorage-common/src/main/java/org/apache/ignite/metastorage/common/Condition.java
##########
@@ -254,11 +263,16 @@ public Condition le(long rev) {
*/
private byte[] val;
+ /**
+ * Key of entry, which will be tested for condition.
Review comment:
Please use one-line Javadoc for such cases.
##########
File path: modules/metastorage-common/src/main/java/org/apache/ignite/metastorage/common/Conditions.java
##########
@@ -40,8 +53,12 @@
* @return Condition on entry value.
* @see Condition.ValueCondition
*/
- public static Condition.ValueCondition value() {
- return new Condition.ValueCondition();
+ public Condition.ValueCondition value() {
+ return new Condition.ValueCondition(key);
+ }
+
+ public static Conditions key(Key key) {
Review comment:
Please add Javadoc here.
##########
File path: modules/metastorage/src/main/java/org/apache/ignite/internal/metastorage/MetaStorageManager.java
##########
@@ -303,15 +306,21 @@ public synchronized void deployWatches() {
}
/**
- * @see MetaStorageService#invoke(Key, Condition, Operation, Operation)
+ * Invoke with single success/failure operation.
+ *
+ * @see MetaStorageService#invoke(Condition, Collection, Collection)
*/
- public @NotNull CompletableFuture<Boolean> invoke(
- @NotNull Key key,
- @NotNull Condition cond,
- @NotNull Operation success,
- @NotNull Operation failure
- ) {
- return metaStorageSvc.invoke(key, cond, success, failure);
+ public @NotNull CompletableFuture<Boolean> invoke(@NotNull Condition cond,
Review comment:
coding guideline does not allow that. please see https://cwiki.apache.org/confluence/display/IGNITE/Coding+Guidelines#CodingGuidelines-MethodArguments
I think it should be as follows:
```
public @NotNull CompletableFuture<Boolean> invoke(
@NotNull Condition cond,
@NotNull Operation success,
@NotNull Operation failure
) {
```
--
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:
users@infra.apache.org
[GitHub] [ignite-3] asfgit closed pull request #113: IGNITE-14670 Update metastorage manager with: range with applied revision, invoke for multiple updates.
Posted by GitBox <gi...@apache.org>.
asfgit closed pull request #113:
URL: https://github.com/apache/ignite-3/pull/113
--
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:
users@infra.apache.org