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 2023/01/12 16:27:19 UTC
[GitHub] [ignite-3] sashapolo opened a new pull request, #1518: IGNITE-18542 Remove some server-side MetaStorage classes
sashapolo opened a new pull request, #1518:
URL: https://github.com/apache/ignite-3/pull/1518
https://issues.apache.org/jira/browse/IGNITE-18542
--
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@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [ignite-3] sashapolo commented on a diff in pull request #1518: IGNITE-18542 Remove some server-side MetaStorage classes
Posted by GitBox <gi...@apache.org>.
sashapolo commented on code in PR #1518:
URL: https://github.com/apache/ignite-3/pull/1518#discussion_r1069126579
##########
modules/metastorage-api/src/main/java/org/apache/ignite/internal/metastorage/dsl/Operation.java:
##########
@@ -17,158 +17,90 @@
package org.apache.ignite.internal.metastorage.dsl;
-import org.jetbrains.annotations.NotNull;
+import org.apache.ignite.internal.util.IgniteUtils;
import org.jetbrains.annotations.Nullable;
/**
* Defines operation for meta storage conditional update (invoke).
*/
public final class Operation {
- /** Actual operation implementation. */
- private final InnerOp upd;
-
/**
- * Constructs an operation which wraps the actual operation implementation.
- *
- * @param upd The actual operation implementation.
+ * Key identifies an entry which operation will be applied to. Key is {@code null} for {@link OperationType#NO_OP} operation.
*/
- Operation(InnerOp upd) {
- this.upd = upd;
- }
+ private final byte @Nullable [] key;
/**
- * Returns actual operation implementation.
+ * Value which will be associated with the {@link #key}. Value is not {@code null} only for {@link OperationType#PUT} operation.
*/
- public InnerOp inner() {
- return upd;
- }
+ private final byte @Nullable [] val;
/**
- * Returns operation type.
+ * Operation type.
*
- * @return Operation type.
+ * @see OperationType
*/
- public OperationType type() {
- return upd.type();
- }
+ private final OperationType type;
/**
- * Represents operation of type <i>remove</i>.
+ * Constructs operation which will be applied to an entry identified by the given key.
+ *
+ * @param type Operation type. Can't be {@code null}.
+ * @param key Key identifies an entry which operation will be applied to.
+ * @param val Value will be associated with an entry identified by the {@code key}.
*/
- public static final class RemoveOp extends AbstractOp {
- /**
- * Default no-op constructor.
- *
- * @param key Identifies an entry which operation will be applied to.
- */
- RemoveOp(byte[] key) {
- super(key, OperationType.REMOVE);
- }
+ public Operation(OperationType type, byte @Nullable [] key, byte @Nullable [] val) {
+ assert (type == OperationType.NO_OP && key == null && val == null)
+ || (type == OperationType.PUT && key != null && val != null)
+ || (type == OperationType.REMOVE && key != null && val == null)
+ : "Invalid operation parameters: [type=" + type
+ + ", key=" + (key == null ? "null" : IgniteUtils.toHexString(key, 256))
+ + ", val=" + (val == null ? "null" : IgniteUtils.toHexString(val, 256)) + ']';
+
+ this.key = key;
+ this.val = val;
+ this.type = type;
}
/**
- * Represents operation of type <i>put</i>.
+ * Returns a key which identifies an entry which operation will be applied to.
+ *
+ * @return A key which identifies an entry which operation will be applied to.
*/
- public static final class PutOp extends AbstractOp {
- /** Value. */
- private final byte[] val;
-
- /**
- * Constructs operation of type <i>put</i>.
- *
- * @param key Identifies an entry which operation will be applied to.
- * @param val The value to which the entry should be updated.
- */
- PutOp(byte[] key, byte[] val) {
- super(key, OperationType.PUT);
-
- this.val = val;
- }
-
- /**
- * Returns value.
- *
- * @return Value.
- */
- public byte[] value() {
- return val;
- }
+ public byte @Nullable [] key() {
+ return key;
}
/**
- * Represents operation of type <i>no-op</i>.
+ * Returns a value which will be associated with an entry identified by the {@code key}.
+ *
+ * @return A value which will be associated with an entry identified by the {@code key}.
*/
- public static final class NoOp extends AbstractOp {
- /**
- * Default no-op constructor.
- */
- NoOp() {
- super(null, OperationType.NO_OP);
- }
+
+ public byte @Nullable [] value() {
+ return val;
}
/**
- * Defines operation interface.
+ * Returns an operation type.
+ *
+ * @return An operation type.
*/
- public interface InnerOp {
- /**
- * Returns key.
- *
- * @return Key.
- */
- @Nullable byte[] key();
-
- /**
- * Returns operation type.
- *
- * @return Operation type.
- */
- @NotNull OperationType type();
+ public OperationType type() {
+ return type;
}
/**
- * Extension of {@link InnerOp}.
+ * Represents operation of type <i>remove</i>.
*/
- private static class AbstractOp implements InnerOp {
- /** Key. */
- @Nullable
- private final byte[] key;
-
- /** Operation type. */
- @NotNull
- private final OperationType type;
-
- /**
- * Ctor.
- *
- * @param key Key.
- * @param type Operation type.
- */
- private AbstractOp(@Nullable byte[] key, OperationType type) {
- this.key = key;
- this.type = type;
- }
+ public static Operation remove(byte[] key) {
+ return new Operation(OperationType.REMOVE, key, null);
+ }
- /**
- * Returns key.
- *
- * @return Key.
- */
- @Nullable
- @Override
- public byte[] key() {
- return key;
- }
+ public static Operation put(byte[] key, byte[] val) {
Review Comment:
I forgot about them, sorry
--
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@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [ignite-3] sashapolo commented on a diff in pull request #1518: IGNITE-18542 Remove some server-side MetaStorage classes
Posted by GitBox <gi...@apache.org>.
sashapolo commented on code in PR #1518:
URL: https://github.com/apache/ignite-3/pull/1518#discussion_r1069122823
##########
modules/metastorage/src/main/java/org/apache/ignite/internal/metastorage/impl/EntryImpl.java:
##########
@@ -19,17 +19,32 @@
import java.util.Arrays;
import org.apache.ignite.internal.metastorage.Entry;
-import org.apache.ignite.lang.ByteArray;
-import org.jetbrains.annotations.NotNull;
+import org.apache.ignite.internal.tostring.S;
import org.jetbrains.annotations.Nullable;
/**
- * Meta storage entry.
+ * Represents a storage unit as entry with key, value and revision.
+ *
+ * <p>Where:
+ * <ul>
+ * <li>key - an unique entry's key represented by an array of bytes. Keys are comparable in lexicographic manner.</li>
+ * <li>value - a data which is associated with a key and represented as an array of bytes.</li>
+ * <li>revision - a number which denotes a version of whole meta storage.
+ * Each change (which could include multiple entries) increments the revision. </li>
+ * <li>updateCounter - a number which increments on every update in the change under one revision.</li>
+ * </ul>
+ *
+ * <p>Instance of {@link #EntryImpl} could represent:
+ * <ul>
+ * <li>A regular entry which stores a particular key, a value and a revision number.</li>
+ * <li>An empty entry which denotes absence a regular entry in the meta storage for a given key.
Review Comment:
These javadocs have been copy-pasted from other class, so they may have some errors)
--
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@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [ignite-3] rpuch commented on a diff in pull request #1518: IGNITE-18542 Remove some server-side MetaStorage classes
Posted by GitBox <gi...@apache.org>.
rpuch commented on code in PR #1518:
URL: https://github.com/apache/ignite-3/pull/1518#discussion_r1069169335
##########
modules/metastorage-api/src/main/java/org/apache/ignite/internal/metastorage/dsl/SimpleCondition.java:
##########
@@ -20,46 +20,101 @@
/**
* Represents a condition for a meta storage conditional update.
*/
-public final class SimpleCondition implements Condition {
- /** Actual condition implementation. */
- private final InnerCondition cond;
+public class SimpleCondition implements Condition {
+ /** Entry key. */
+ private final byte[] key;
/**
- * Constructs a condition, which wraps the actual condition implementation.
- *
- * @param cond The actual condition implementation.
+ * Condition type.
*/
- SimpleCondition(InnerCondition cond) {
- this.cond = cond;
+ private final ConditionType type;
+
+ private SimpleCondition(byte[] key, ConditionType type) {
+ this.key = key;
+ this.type = type;
}
- public InnerCondition inner() {
- return cond;
+ /**
+ * Returns the key, which identifies an entry, which condition will be applied to.
+ *
+ * @return Key, which identifies an entry, which condition will be applied to.
+ */
+ public byte[] key() {
+ return key;
}
+ /**
+ * Returns the condition type.
+ *
+ * @return Condition type.
+ */
public ConditionType type() {
- return cond.type();
+ return type;
}
/**
- * Represents a condition on an entry revision. Only one type of condition could be applied to the one instance of a condition.
- * Subsequent invocations of any method, which produces a condition will throw {@link IllegalStateException}.
+ * Creates a builder for a revision condition.
+ *
+ * @param key Key, which identifies an entry, which condition will be applied to.
+ * @return Builder for a revision condition.
+ * @see RevisionCondition
*/
- public static final class RevisionCondition extends AbstractCondition {
- /** The revision as the condition argument. */
- private long rev;
+ public static RevisionConditionBuilder revision(byte[] key) {
+ return new RevisionConditionBuilder(key);
+ }
- /**
- * Constructs a condition by a revision for an entry identified by the given key.
- *
- * @param key Identifies an entry, which condition will be applied to.
- */
- RevisionCondition(byte[] key) {
- super(key);
- }
+ /**
+ * Creates a builder for a value condition.
+ *
+ * @param key Key, which identifies an entry, which condition will be applied to.
+ * @return Builder for a value condition.
+ * @see ValueCondition
+ */
+ public static ValueConditionBuilder value(byte[] key) {
+ return new ValueConditionBuilder(key);
+ }
- public long revision() {
- return rev;
+ /**
+ * Produces the condition of type {@link ConditionType#TOMBSTONE}. This condition tests that an entry's value, identified by the
+ * given key, is tombstone.
+ *
+ * @return The condition of type {@link ConditionType#TOMBSTONE}.
+ * @throws IllegalStateException In the case when the condition is already defined.
+ */
+ public static SimpleCondition tombstone(byte[] key) {
+ return new SimpleCondition(key, ConditionType.TOMBSTONE);
+ }
+
+ /**
+ * Produces the condition of type {@link ConditionType#KEY_EXISTS}. This condition tests the existence of an entry identified by the
+ * given key.
+ *
+ * @return The condition of type {@link ConditionType#KEY_EXISTS}.
+ * @throws IllegalStateException In the case when the condition is already defined.
Review Comment:
Do we need this?
##########
modules/metastorage-api/src/main/java/org/apache/ignite/internal/metastorage/dsl/SimpleCondition.java:
##########
@@ -20,46 +20,101 @@
/**
* Represents a condition for a meta storage conditional update.
*/
-public final class SimpleCondition implements Condition {
- /** Actual condition implementation. */
- private final InnerCondition cond;
+public class SimpleCondition implements Condition {
+ /** Entry key. */
+ private final byte[] key;
/**
- * Constructs a condition, which wraps the actual condition implementation.
- *
- * @param cond The actual condition implementation.
+ * Condition type.
*/
- SimpleCondition(InnerCondition cond) {
- this.cond = cond;
+ private final ConditionType type;
+
+ private SimpleCondition(byte[] key, ConditionType type) {
+ this.key = key;
+ this.type = type;
}
- public InnerCondition inner() {
- return cond;
+ /**
+ * Returns the key, which identifies an entry, which condition will be applied to.
+ *
+ * @return Key, which identifies an entry, which condition will be applied to.
+ */
+ public byte[] key() {
+ return key;
}
+ /**
+ * Returns the condition type.
+ *
+ * @return Condition type.
+ */
public ConditionType type() {
- return cond.type();
+ return type;
}
/**
- * Represents a condition on an entry revision. Only one type of condition could be applied to the one instance of a condition.
- * Subsequent invocations of any method, which produces a condition will throw {@link IllegalStateException}.
+ * Creates a builder for a revision condition.
+ *
+ * @param key Key, which identifies an entry, which condition will be applied to.
+ * @return Builder for a revision condition.
+ * @see RevisionCondition
*/
- public static final class RevisionCondition extends AbstractCondition {
- /** The revision as the condition argument. */
- private long rev;
+ public static RevisionConditionBuilder revision(byte[] key) {
+ return new RevisionConditionBuilder(key);
+ }
- /**
- * Constructs a condition by a revision for an entry identified by the given key.
- *
- * @param key Identifies an entry, which condition will be applied to.
- */
- RevisionCondition(byte[] key) {
- super(key);
- }
+ /**
+ * Creates a builder for a value condition.
+ *
+ * @param key Key, which identifies an entry, which condition will be applied to.
+ * @return Builder for a value condition.
+ * @see ValueCondition
+ */
+ public static ValueConditionBuilder value(byte[] key) {
+ return new ValueConditionBuilder(key);
+ }
- public long revision() {
- return rev;
+ /**
+ * Produces the condition of type {@link ConditionType#TOMBSTONE}. This condition tests that an entry's value, identified by the
+ * given key, is tombstone.
+ *
+ * @return The condition of type {@link ConditionType#TOMBSTONE}.
+ * @throws IllegalStateException In the case when the condition is already defined.
Review Comment:
I don't see a possibility for this exception to be thrown. Why do we need this declaration?
##########
modules/metastorage-api/src/main/java/org/apache/ignite/internal/metastorage/dsl/SimpleCondition.java:
##########
@@ -91,15 +138,7 @@ public SimpleCondition eq(long rev) {
* @throws IllegalStateException In the case when the condition is already defined.
Review Comment:
Do we need this?
##########
modules/metastorage-api/src/main/java/org/apache/ignite/internal/metastorage/dsl/SimpleCondition.java:
##########
@@ -20,46 +20,101 @@
/**
* Represents a condition for a meta storage conditional update.
*/
-public final class SimpleCondition implements Condition {
- /** Actual condition implementation. */
- private final InnerCondition cond;
+public class SimpleCondition implements Condition {
+ /** Entry key. */
+ private final byte[] key;
/**
- * Constructs a condition, which wraps the actual condition implementation.
- *
- * @param cond The actual condition implementation.
+ * Condition type.
*/
- SimpleCondition(InnerCondition cond) {
- this.cond = cond;
+ private final ConditionType type;
+
+ private SimpleCondition(byte[] key, ConditionType type) {
+ this.key = key;
+ this.type = type;
}
- public InnerCondition inner() {
- return cond;
+ /**
+ * Returns the key, which identifies an entry, which condition will be applied to.
+ *
+ * @return Key, which identifies an entry, which condition will be applied to.
+ */
+ public byte[] key() {
+ return key;
}
+ /**
+ * Returns the condition type.
+ *
+ * @return Condition type.
+ */
public ConditionType type() {
- return cond.type();
+ return type;
}
/**
- * Represents a condition on an entry revision. Only one type of condition could be applied to the one instance of a condition.
- * Subsequent invocations of any method, which produces a condition will throw {@link IllegalStateException}.
+ * Creates a builder for a revision condition.
+ *
+ * @param key Key, which identifies an entry, which condition will be applied to.
+ * @return Builder for a revision condition.
+ * @see RevisionCondition
*/
- public static final class RevisionCondition extends AbstractCondition {
- /** The revision as the condition argument. */
- private long rev;
+ public static RevisionConditionBuilder revision(byte[] key) {
+ return new RevisionConditionBuilder(key);
+ }
- /**
- * Constructs a condition by a revision for an entry identified by the given key.
- *
- * @param key Identifies an entry, which condition will be applied to.
- */
- RevisionCondition(byte[] key) {
- super(key);
- }
+ /**
+ * Creates a builder for a value condition.
+ *
+ * @param key Key, which identifies an entry, which condition will be applied to.
+ * @return Builder for a value condition.
+ * @see ValueCondition
+ */
+ public static ValueConditionBuilder value(byte[] key) {
+ return new ValueConditionBuilder(key);
+ }
- public long revision() {
- return rev;
+ /**
+ * Produces the condition of type {@link ConditionType#TOMBSTONE}. This condition tests that an entry's value, identified by the
+ * given key, is tombstone.
+ *
+ * @return The condition of type {@link ConditionType#TOMBSTONE}.
+ * @throws IllegalStateException In the case when the condition is already defined.
+ */
+ public static SimpleCondition tombstone(byte[] key) {
+ return new SimpleCondition(key, ConditionType.TOMBSTONE);
+ }
+
+ /**
+ * Produces the condition of type {@link ConditionType#KEY_EXISTS}. This condition tests the existence of an entry identified by the
+ * given key.
+ *
+ * @return The condition of type {@link ConditionType#KEY_EXISTS}.
+ * @throws IllegalStateException In the case when the condition is already defined.
+ */
+ public static SimpleCondition exists(byte[] key) {
+ return new SimpleCondition(key, ConditionType.KEY_EXISTS);
+ }
+
+ /**
+ * Produces the condition of type {@link ConditionType#KEY_NOT_EXISTS}. This condition tests the non-existence of an entry
+ * identified by the given key.
+ *
+ * @return The condition of type {@link ConditionType#KEY_NOT_EXISTS}.
+ * @throws IllegalStateException In the case when the condition is already defined.
Review Comment:
Do we need this?
##########
modules/metastorage-api/src/main/java/org/apache/ignite/internal/metastorage/dsl/SimpleCondition.java:
##########
@@ -71,15 +126,7 @@ public long revision() {
* @throws IllegalStateException In the case when the condition is already defined.
Review Comment:
Do we need 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@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [ignite-3] rpuch commented on a diff in pull request #1518: IGNITE-18542 Remove some server-side MetaStorage classes
Posted by GitBox <gi...@apache.org>.
rpuch commented on code in PR #1518:
URL: https://github.com/apache/ignite-3/pull/1518#discussion_r1068984477
##########
modules/metastorage-api/src/main/java/org/apache/ignite/internal/metastorage/dsl/Operation.java:
##########
@@ -17,158 +17,90 @@
package org.apache.ignite.internal.metastorage.dsl;
-import org.jetbrains.annotations.NotNull;
+import org.apache.ignite.internal.util.IgniteUtils;
import org.jetbrains.annotations.Nullable;
/**
* Defines operation for meta storage conditional update (invoke).
*/
public final class Operation {
- /** Actual operation implementation. */
- private final InnerOp upd;
-
/**
- * Constructs an operation which wraps the actual operation implementation.
- *
- * @param upd The actual operation implementation.
+ * Key identifies an entry which operation will be applied to. Key is {@code null} for {@link OperationType#NO_OP} operation.
*/
- Operation(InnerOp upd) {
- this.upd = upd;
- }
+ private final byte @Nullable [] key;
/**
- * Returns actual operation implementation.
+ * Value which will be associated with the {@link #key}. Value is not {@code null} only for {@link OperationType#PUT} operation.
*/
- public InnerOp inner() {
- return upd;
- }
+ private final byte @Nullable [] val;
/**
- * Returns operation type.
+ * Operation type.
*
- * @return Operation type.
+ * @see OperationType
*/
- public OperationType type() {
- return upd.type();
- }
+ private final OperationType type;
/**
- * Represents operation of type <i>remove</i>.
+ * Constructs operation which will be applied to an entry identified by the given key.
+ *
+ * @param type Operation type. Can't be {@code null}.
+ * @param key Key identifies an entry which operation will be applied to.
+ * @param val Value will be associated with an entry identified by the {@code key}.
*/
- public static final class RemoveOp extends AbstractOp {
- /**
- * Default no-op constructor.
- *
- * @param key Identifies an entry which operation will be applied to.
- */
- RemoveOp(byte[] key) {
- super(key, OperationType.REMOVE);
- }
+ public Operation(OperationType type, byte @Nullable [] key, byte @Nullable [] val) {
+ assert (type == OperationType.NO_OP && key == null && val == null)
+ || (type == OperationType.PUT && key != null && val != null)
+ || (type == OperationType.REMOVE && key != null && val == null)
+ : "Invalid operation parameters: [type=" + type
+ + ", key=" + (key == null ? "null" : IgniteUtils.toHexString(key, 256))
+ + ", val=" + (val == null ? "null" : IgniteUtils.toHexString(val, 256)) + ']';
+
+ this.key = key;
+ this.val = val;
+ this.type = type;
}
/**
- * Represents operation of type <i>put</i>.
+ * Returns a key which identifies an entry which operation will be applied to.
+ *
+ * @return A key which identifies an entry which operation will be applied to.
*/
- public static final class PutOp extends AbstractOp {
- /** Value. */
- private final byte[] val;
-
- /**
- * Constructs operation of type <i>put</i>.
- *
- * @param key Identifies an entry which operation will be applied to.
- * @param val The value to which the entry should be updated.
- */
- PutOp(byte[] key, byte[] val) {
- super(key, OperationType.PUT);
-
- this.val = val;
- }
-
- /**
- * Returns value.
- *
- * @return Value.
- */
- public byte[] value() {
- return val;
- }
+ public byte @Nullable [] key() {
+ return key;
}
/**
- * Represents operation of type <i>no-op</i>.
+ * Returns a value which will be associated with an entry identified by the {@code key}.
+ *
+ * @return A value which will be associated with an entry identified by the {@code key}.
*/
- public static final class NoOp extends AbstractOp {
- /**
- * Default no-op constructor.
- */
- NoOp() {
- super(null, OperationType.NO_OP);
- }
+
+ public byte @Nullable [] value() {
+ return val;
}
/**
- * Defines operation interface.
+ * Returns an operation type.
+ *
+ * @return An operation type.
*/
- public interface InnerOp {
- /**
- * Returns key.
- *
- * @return Key.
- */
- @Nullable byte[] key();
-
- /**
- * Returns operation type.
- *
- * @return Operation type.
- */
- @NotNull OperationType type();
+ public OperationType type() {
+ return type;
}
/**
- * Extension of {@link InnerOp}.
+ * Represents operation of type <i>remove</i>.
*/
- private static class AbstractOp implements InnerOp {
- /** Key. */
- @Nullable
- private final byte[] key;
-
- /** Operation type. */
- @NotNull
- private final OperationType type;
-
- /**
- * Ctor.
- *
- * @param key Key.
- * @param type Operation type.
- */
- private AbstractOp(@Nullable byte[] key, OperationType type) {
- this.key = key;
- this.type = type;
- }
+ public static Operation remove(byte[] key) {
+ return new Operation(OperationType.REMOVE, key, null);
+ }
- /**
- * Returns key.
- *
- * @return Key.
- */
- @Nullable
- @Override
- public byte[] key() {
- return key;
- }
+ public static Operation put(byte[] key, byte[] val) {
Review Comment:
`put()` and `noOp()` do not have any javadoc, when `remove()` does have one. Is this intentional?
##########
modules/metastorage-api/src/main/java/org/apache/ignite/internal/metastorage/dsl/SimpleCondition.java:
##########
@@ -20,46 +20,82 @@
/**
* Represents a condition for a meta storage conditional update.
*/
-public final class SimpleCondition implements Condition {
- /** Actual condition implementation. */
- private final InnerCondition cond;
+public class SimpleCondition implements Condition {
+ /** Entry key. */
+ private final byte[] key;
/**
- * Constructs a condition, which wraps the actual condition implementation.
- *
- * @param cond The actual condition implementation.
+ * Condition type.
*/
- SimpleCondition(InnerCondition cond) {
- this.cond = cond;
+ private final ConditionType type;
+
+ private SimpleCondition(byte[] key, ConditionType type) {
+ this.key = key;
+ this.type = type;
}
- public InnerCondition inner() {
- return cond;
+ /**
+ * Returns the key, which identifies an entry, which condition will be applied to.
+ *
+ * @return Key, which identifies an entry, which condition will be applied to.
+ */
+ public byte[] key() {
+ return key;
}
public ConditionType type() {
- return cond.type();
+ return type;
+ }
+
+ public static RevisionConditionBuilder revision(byte[] key) {
Review Comment:
How about a javadoc?
##########
modules/metastorage/src/test/java/org/apache/ignite/internal/metastorage/watch/WatchAggregatorTest.java:
##########
@@ -274,8 +274,8 @@ private Entry entry(String key, String val, long revision, long updateCntr) {
return new Entry() {
/** {@inheritDoc} */
@Override
- public @NotNull ByteArray key() {
- return new ByteArray(key);
+ public @NotNull byte[] key() {
Review Comment:
Let's remove `@NotNull`
##########
modules/metastorage-api/src/main/java/org/apache/ignite/internal/metastorage/dsl/SimpleCondition.java:
##########
@@ -304,155 +277,31 @@ public SimpleCondition lt(byte[] val) {
* @throws IllegalStateException In the case when the condition is already defined.
*/
public SimpleCondition le(byte[] val) {
- validate(type());
-
- type(ConditionType.VAL_LESS_OR_EQUAL);
-
- this.val = val;
-
- return new SimpleCondition(this);
+ return new ValueCondition(key, ConditionType.VAL_LESS_OR_EQUAL, val);
}
}
/**
- * Represents a condition on an entry existence. Only the one type of a condition could be applied to the one instance of a condition.
+ * Represents a condition on an entry value. Only the one type of condition could be applied to the one instance of a condition.
Review Comment:
```suggestion
* Represents a condition on an entry value. Only one type of condition could be applied to one instance of a condition.
```
##########
modules/runner/src/main/java/org/apache/ignite/internal/configuration/storage/DistributedConfigurationStorage.java:
##########
@@ -160,11 +163,11 @@ public void close() {
// Meta Storage should not return nulls as values
assert value != null;
- if (key.equals(MASTER_KEY)) {
+ if (Arrays.equals(key, MASTER_KEY.bytes())) {
continue;
}
- String dataKey = key.toString().substring(DISTRIBUTED_PREFIX.length());
+ String dataKey = new String(key, StandardCharsets.UTF_8).substring(DISTRIBUTED_PREFIX.length());
Review Comment:
`UTF_8` can be imported statically decreasing the amount of syntactic noise: it is not that important that it goes from `StandardCharsets`; the important piece of information here is that it's utf-8.
##########
modules/metastorage-api/src/main/java/org/apache/ignite/internal/metastorage/Entry.java:
##########
@@ -37,7 +35,7 @@ public interface Entry {
*
* @return The key.
*/
- @NotNull ByteArray key();
+ byte[] key();
Review Comment:
Why was `ByteArray` changed to `byte[]` here? `ByteArray` seemed to only add equals/comparison support; isn't it useful anymore?
##########
modules/metastorage/src/main/java/org/apache/ignite/internal/metastorage/impl/EntryImpl.java:
##########
@@ -19,17 +19,32 @@
import java.util.Arrays;
import org.apache.ignite.internal.metastorage.Entry;
-import org.apache.ignite.lang.ByteArray;
-import org.jetbrains.annotations.NotNull;
+import org.apache.ignite.internal.tostring.S;
import org.jetbrains.annotations.Nullable;
/**
- * Meta storage entry.
+ * Represents a storage unit as entry with key, value and revision.
+ *
+ * <p>Where:
+ * <ul>
+ * <li>key - an unique entry's key represented by an array of bytes. Keys are comparable in lexicographic manner.</li>
+ * <li>value - a data which is associated with a key and represented as an array of bytes.</li>
+ * <li>revision - a number which denotes a version of whole meta storage.
+ * Each change (which could include multiple entries) increments the revision. </li>
+ * <li>updateCounter - a number which increments on every update in the change under one revision.</li>
+ * </ul>
+ *
+ * <p>Instance of {@link #EntryImpl} could represent:
+ * <ul>
+ * <li>A regular entry which stores a particular key, a value and a revision number.</li>
+ * <li>An empty entry which denotes absence a regular entry in the meta storage for a given key.
Review Comment:
```suggestion
* <li>An empty entry which denotes absence of a regular entry in the meta storage for a given key.
```
##########
modules/metastorage-api/src/main/java/org/apache/ignite/internal/metastorage/dsl/SimpleCondition.java:
##########
@@ -171,37 +167,44 @@ public SimpleCondition lt(long rev) {
* @throws IllegalStateException In the case when the condition is already defined.
*/
public SimpleCondition le(long rev) {
- assert rev > 0 : "Revision must be positive.";
-
- validate(type());
-
- type(ConditionType.REV_LESS_OR_EQUAL);
-
- this.rev = rev;
-
- return new SimpleCondition(this);
+ return new RevisionCondition(key, ConditionType.REV_LESS_OR_EQUAL, rev);
}
}
/**
- * Represents a condition on an entry value. Only the one type of condition could be applied to the one instance of a condition.
+ * Represents a condition on an entry revision. Only one type of condition could be applied to the one instance of a condition.
Review Comment:
```suggestion
* Represents a condition on an entry revision. Only one type of condition could be applied to one instance of a condition.
```
--
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@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [ignite-3] sashapolo commented on a diff in pull request #1518: IGNITE-18542 Remove some server-side MetaStorage classes
Posted by GitBox <gi...@apache.org>.
sashapolo commented on code in PR #1518:
URL: https://github.com/apache/ignite-3/pull/1518#discussion_r1069123280
##########
modules/metastorage/src/test/java/org/apache/ignite/internal/metastorage/watch/WatchAggregatorTest.java:
##########
@@ -274,8 +274,8 @@ private Entry entry(String key, String val, long revision, long updateCntr) {
return new Entry() {
/** {@inheritDoc} */
@Override
- public @NotNull ByteArray key() {
- return new ByteArray(key);
+ public @NotNull byte[] key() {
Review Comment:
This class will be removed altogether in my next PR, so I don't think it's necessary
##########
modules/runner/src/main/java/org/apache/ignite/internal/configuration/storage/DistributedConfigurationStorage.java:
##########
@@ -160,11 +163,11 @@ public void close() {
// Meta Storage should not return nulls as values
assert value != null;
- if (key.equals(MASTER_KEY)) {
+ if (Arrays.equals(key, MASTER_KEY.bytes())) {
continue;
}
- String dataKey = key.toString().substring(DISTRIBUTED_PREFIX.length());
+ String dataKey = new String(key, StandardCharsets.UTF_8).substring(DISTRIBUTED_PREFIX.length());
Review Comment:
sure
--
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@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [ignite-3] sashapolo commented on a diff in pull request #1518: IGNITE-18542 Remove some server-side MetaStorage classes
Posted by GitBox <gi...@apache.org>.
sashapolo commented on code in PR #1518:
URL: https://github.com/apache/ignite-3/pull/1518#discussion_r1069126156
##########
modules/metastorage-api/src/main/java/org/apache/ignite/internal/metastorage/Entry.java:
##########
@@ -37,7 +35,7 @@ public interface Entry {
*
* @return The key.
*/
- @NotNull ByteArray key();
+ byte[] key();
Review Comment:
`ByteArray` doesn't make much sense, I think it needs to be removed altogether. First, its just unnecessary wrapping and object creation (you can compare arrays without it). Secondly, it makes an incorrect assumption that arrays are compared using signed comparison, however we use unsigned comparison in all storages
--
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@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [ignite-3] tkalkirill merged pull request #1518: IGNITE-18542 Remove some server-side MetaStorage classes
Posted by GitBox <gi...@apache.org>.
tkalkirill merged PR #1518:
URL: https://github.com/apache/ignite-3/pull/1518
--
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@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org