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 2020/02/07 09:49:43 UTC
[GitHub] [ignite] ibessonov opened a new pull request #7383: IGNITE-12638
DMS classes are IgniteDataTransferObjects now.
ibessonov opened a new pull request #7383: IGNITE-12638 DMS classes are IgniteDataTransferObjects now.
URL: https://github.com/apache/ignite/pull/7383
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [ignite] agura commented on a change in pull request #7383:
IGNITE-12638 DMS classes are IgniteDataTransferObjects now.
Posted by GitBox <gi...@apache.org>.
agura commented on a change in pull request #7383: IGNITE-12638 DMS classes are IgniteDataTransferObjects now.
URL: https://github.com/apache/ignite/pull/7383#discussion_r378331793
##########
File path: modules/core/src/main/java/org/apache/ignite/internal/processors/metastorage/persistence/DistributedMetaStorageVersion.java
##########
@@ -133,6 +141,19 @@ public DistributedMetaStorageVersion nextVersion(
return new DistributedMetaStorageVersion(id + toVer + 1 - fromVer, hash);
}
+ /** {@inheritDoc} */
+ @Override protected void writeExternalData(ObjectOutput out) throws IOException {
+ out.writeLong(id);
+ out.writeLong(hash);
+ }
+
+ /** {@inheritDoc} */
+ @Override
Review comment:
`@Override` should be on the same line as method signature.
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [ignite] ibessonov closed pull request #7383: IGNITE-12638 DMS
classes are IgniteDataTransferObjects now.
Posted by GitBox <gi...@apache.org>.
ibessonov closed pull request #7383: IGNITE-12638 DMS classes are IgniteDataTransferObjects now.
URL: https://github.com/apache/ignite/pull/7383
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [ignite] agura commented on a change in pull request #7383:
IGNITE-12638 DMS classes are IgniteDataTransferObjects now.
Posted by GitBox <gi...@apache.org>.
agura commented on a change in pull request #7383: IGNITE-12638 DMS classes are IgniteDataTransferObjects now.
URL: https://github.com/apache/ignite/pull/7383#discussion_r378329256
##########
File path: modules/core/src/main/java/org/apache/ignite/internal/processors/metastorage/persistence/DistributedMetaStorageHistoryItem.java
##########
@@ -33,15 +37,19 @@
/** */
@GridToStringInclude
- public final String[] keys;
+ public String[] keys;
/** */
@GridToStringInclude
- public final byte[][] valBytesArray;
+ public byte[][] valBytesArray;
/** */
private transient long longHash;
+ /** Default constructor for deserialization. */
+ public DistributedMetaStorageHistoryItem() {
+ }
Review comment:
Please, add `// No-op.` comment into the body of constructor.
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [ignite] agura commented on a change in pull request #7383:
IGNITE-12638 DMS classes are IgniteDataTransferObjects now.
Posted by GitBox <gi...@apache.org>.
agura commented on a change in pull request #7383: IGNITE-12638 DMS classes are IgniteDataTransferObjects now.
URL: https://github.com/apache/ignite/pull/7383#discussion_r378329778
##########
File path: modules/core/src/main/java/org/apache/ignite/internal/processors/metastorage/persistence/DistributedMetaStorageHistoryItem.java
##########
@@ -68,6 +78,31 @@ public long estimateSize() {
return size;
}
+ /** {@inheritDoc} */
+ @Override protected void writeExternalData(ObjectOutput out) throws IOException {
+ out.writeInt(keys.length);
+
+ for (int i = 0; i < keys.length; i++) {
+ U.writeString(out, keys[i]);
+
+ U.writeByteArray(out, valBytesArray[i]);
+ }
+ }
+
+ /** {@inheritDoc} */
+ @Override
Review comment:
`@Override` should be on the same line as method signature.
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [ignite] agura commented on a change in pull request #7383:
IGNITE-12638 DMS classes are IgniteDataTransferObjects now.
Posted by GitBox <gi...@apache.org>.
agura commented on a change in pull request #7383: IGNITE-12638 DMS classes are IgniteDataTransferObjects now.
URL: https://github.com/apache/ignite/pull/7383#discussion_r378332930
##########
File path: modules/core/src/main/java/org/apache/ignite/internal/processors/metastorage/persistence/DistributedMetaStorageHistoryItem.java
##########
@@ -50,6 +58,8 @@ public DistributedMetaStorageHistoryItem(String key, byte[] valBytes) {
/** */
public DistributedMetaStorageHistoryItem(String[] keys, byte[][] valBytesArray) {
+ assert keys.length == valBytesArray.length;
Review comment:
Abbreviation should be used for `valBytesArray` parameter and corresponding class field.
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [ignite] agura commented on a change in pull request #7383:
IGNITE-12638 DMS classes are IgniteDataTransferObjects now.
Posted by GitBox <gi...@apache.org>.
agura commented on a change in pull request #7383: IGNITE-12638 DMS classes are IgniteDataTransferObjects now.
URL: https://github.com/apache/ignite/pull/7383#discussion_r378330389
##########
File path: modules/core/src/main/java/org/apache/ignite/internal/processors/metastorage/persistence/DistributedMetaStorageHistoryItem.java
##########
@@ -68,6 +78,31 @@ public long estimateSize() {
return size;
}
+ /** {@inheritDoc} */
+ @Override protected void writeExternalData(ObjectOutput out) throws IOException {
+ out.writeInt(keys.length);
+
+ for (int i = 0; i < keys.length; i++) {
+ U.writeString(out, keys[i]);
+
+ U.writeByteArray(out, valBytesArray[i]);
+ }
+ }
+
+ /** {@inheritDoc} */
+ @Override
+ protected void readExternalData(byte protoVer, ObjectInput in) throws IOException {
+ int len = in.readInt();
+
+ keys = new String[len];
Review comment:
Do we really need empty arrays in case if `len == 0`?
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [ignite] agura commented on a change in pull request #7383:
IGNITE-12638 DMS classes are IgniteDataTransferObjects now.
Posted by GitBox <gi...@apache.org>.
agura commented on a change in pull request #7383: IGNITE-12638 DMS classes are IgniteDataTransferObjects now.
URL: https://github.com/apache/ignite/pull/7383#discussion_r378330805
##########
File path: modules/core/src/main/java/org/apache/ignite/internal/processors/metastorage/persistence/DistributedMetaStorageVersion.java
##########
@@ -44,14 +48,18 @@ private static long nextHash(long hash, DistributedMetaStorageHistoryItem update
* @see #INITIAL_VERSION
*/
@GridToStringInclude
- public final long id;
+ public long id;
/**
* Hash of the whole updates list. Hashing algorinthm is almost the same as in {@link List#hashCode()}, but with
* {@code long} value instead of {@code int}.
*/
@GridToStringInclude
- public final long hash;
+ public long hash;
+
+ /** Default constructor for deserialization. */
+ public DistributedMetaStorageVersion() {
+ }
Review comment:
Please, add `// No-op.` comment into the body of constructor.
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [ignite] ibessonov commented on a change in pull request #7383:
IGNITE-12638 DMS classes are IgniteDataTransferObjects now.
Posted by GitBox <gi...@apache.org>.
ibessonov commented on a change in pull request #7383: IGNITE-12638 DMS classes are IgniteDataTransferObjects now.
URL: https://github.com/apache/ignite/pull/7383#discussion_r378664323
##########
File path: modules/core/src/main/java/org/apache/ignite/internal/processors/metastorage/persistence/DistributedMetaStorageHistoryItem.java
##########
@@ -68,6 +78,31 @@ public long estimateSize() {
return size;
}
+ /** {@inheritDoc} */
+ @Override protected void writeExternalData(ObjectOutput out) throws IOException {
+ out.writeInt(keys.length);
+
+ for (int i = 0; i < keys.length; i++) {
+ U.writeString(out, keys[i]);
+
+ U.writeByteArray(out, valBytesArray[i]);
+ }
+ }
+
+ /** {@inheritDoc} */
+ @Override
+ protected void readExternalData(byte protoVer, ObjectInput in) throws IOException {
+ int len = in.readInt();
+
+ keys = new String[len];
Review comment:
len == 0 is impossible, I'll assert it in the constructor.
----------------------------------------------------------------
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
With regards,
Apache Git Services