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/04/02 12:28:09 UTC
[GitHub] [ignite] tkalkirill opened a new pull request #7613: IGNITE-12821
Add check size into validate_indexes
tkalkirill opened a new pull request #7613: IGNITE-12821 Add check size into validate_indexes
URL: https://github.com/apache/ignite/pull/7613
----------------------------------------------------------------
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] tkalkirill commented on a change in pull request #7613:
IGNITE-12821 Add check size into validate_indexes
Posted by GitBox <gi...@apache.org>.
tkalkirill commented on a change in pull request #7613: IGNITE-12821 Add check size into validate_indexes
URL: https://github.com/apache/ignite/pull/7613#discussion_r407590054
##########
File path: modules/indexing/src/main/java/org/apache/ignite/internal/visor/verify/ValidateIndexesClosure.java
##########
@@ -206,53 +230,55 @@ private VisorValidateIndexesJobResult call0() {
List<Future<Map<PartitionKey, ValidateIndexesPartitionResult>>> procPartFutures = new ArrayList<>();
List<Future<Map<String, ValidateIndexesPartitionResult>>> procIdxFutures = new ArrayList<>();
+ List<T3<CacheGroupContext, GridDhtLocalPartition, Future<CacheSize>>> cacheSizeFutures = new ArrayList<>();
+ List<T3<GridCacheContext, Index, Future<T2<Throwable, Long>>>> idxSizeFutures = new ArrayList<>();
Review comment:
Can I not 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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [ignite] sk0x50 commented on a change in pull request #7613:
IGNITE-12821 Add check size into validate_indexes
Posted by GitBox <gi...@apache.org>.
sk0x50 commented on a change in pull request #7613: IGNITE-12821 Add check size into validate_indexes
URL: https://github.com/apache/ignite/pull/7613#discussion_r407258745
##########
File path: modules/indexing/src/main/java/org/apache/ignite/internal/visor/verify/ValidateIndexesClosure.java
##########
@@ -206,53 +230,55 @@ private VisorValidateIndexesJobResult call0() {
List<Future<Map<PartitionKey, ValidateIndexesPartitionResult>>> procPartFutures = new ArrayList<>();
List<Future<Map<String, ValidateIndexesPartitionResult>>> procIdxFutures = new ArrayList<>();
+ List<T3<CacheGroupContext, GridDhtLocalPartition, Future<CacheSize>>> cacheSizeFutures = new ArrayList<>();
+ List<T3<GridCacheContext, Index, Future<T2<Throwable, Long>>>> idxSizeFutures = new ArrayList<>();
+
List<T2<CacheGroupContext, GridDhtLocalPartition>> partArgs = new ArrayList<>();
List<T2<GridCacheContext, Index>> idxArgs = new ArrayList<>();
totalCacheGrps = grpIds.size();
Map<Integer, IndexIntegrityCheckIssue> integrityCheckResults = integrityCheckIndexesPartitions(grpIds);
+ GridQueryProcessor qryProcessor = ignite.context().query();
+ IgniteH2Indexing h2Indexing = (IgniteH2Indexing)qryProcessor.getIndexing();
+
for (Integer grpId : grpIds) {
CacheGroupContext grpCtx = ignite.context().cache().cacheGroup(grpId);
- if (grpCtx == null || integrityCheckResults.containsKey(grpId))
+ if (isNull(grpCtx) || integrityCheckResults.containsKey(grpId))
Review comment:
This is arguable of course, but I see no benefit from using *isNull* instead of *== null*.
Moreover, the javadoc states the following:
> @apiNote This method exists to be used as a {@link java.util.function.Predicate}, {@code filter(Objects::isNull)}
----------------------------------------------------------------
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] sk0x50 commented on a change in pull request #7613:
IGNITE-12821 Add check size into validate_indexes
Posted by GitBox <gi...@apache.org>.
sk0x50 commented on a change in pull request #7613: IGNITE-12821 Add check size into validate_indexes
URL: https://github.com/apache/ignite/pull/7613#discussion_r407257408
##########
File path: modules/core/src/main/java/org/apache/ignite/internal/visor/verify/VisorValidateIndexesTaskArg.java
##########
@@ -101,49 +122,47 @@ public int getCheckFirst() {
}
/**
- * @return checkThrough.
+ * Returns whether to check that index size and cache size are same.
+ *
+ * @return {@code true} if need check that index size and cache size
+ * are same.
*/
- public int getCheckThrough() {
- return checkThrough;
+ public boolean checkSizes() {
+ return checkSizes;
}
/** {@inheritDoc} */
@Override protected void writeExternalData(ObjectOutput out) throws IOException {
- U.writeCollection(out, caches);
+ writeCollection(out, caches);
out.writeInt(checkFirst);
out.writeInt(checkThrough);
- U.writeCollection(out, nodes);
+ writeCollection(out, nodes);
out.writeBoolean(checkCrc);
+ out.writeBoolean(checkSizes);
}
/** {@inheritDoc} */
@Override protected void readExternalData(byte protoVer, ObjectInput in) throws IOException, ClassNotFoundException {
- caches = U.readSet(in);
+ caches = readSet(in);
if (protoVer > V1) {
checkFirst = in.readInt();
checkThrough = in.readInt();
}
- else {
- checkFirst = -1;
- checkThrough = -1;
- }
if (protoVer > V2)
- nodes = U.readSet(in);
+ nodes = readSet(in);
- if (protoVer >= V6)
+ if (protoVer > V5)
checkCrc = in.readBoolean();
- }
- /** Set checkCrc */
- protected void checkCrc(boolean checkCrc) {
- this.checkCrc = checkCrc;
+ if (protoVer > V6)
+ checkSizes = in.readBoolean();
}
/** {@inheritDoc} */
@Override public byte getProtocolVersion() {
- return V6;
+ return V7;
Review comment:
Do we really need to try to support the rolling upgrade 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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [ignite] tkalkirill commented on a change in pull request #7613:
IGNITE-12821 Add check size into validate_indexes
Posted by GitBox <gi...@apache.org>.
tkalkirill commented on a change in pull request #7613: IGNITE-12821 Add check size into validate_indexes
URL: https://github.com/apache/ignite/pull/7613#discussion_r407588749
##########
File path: modules/indexing/src/main/java/org/apache/ignite/internal/visor/verify/ValidateIndexesClosure.java
##########
@@ -412,7 +476,7 @@ private IndexIntegrityCheckIssue integrityCheckIndexPartition(CacheGroupContext
finally {
integrityCheckedIndexes.incrementAndGet();
- printProgressIfNeeded("Current progress of ValidateIndexesClosure: checked integrity of "
+ printProgressIfNeeded(() -> "Current progress of ValidateIndexesClosure: checked integrity of "
Review comment:
Yes, plus there is a lambda cache and string is not created if not needed.
----------------------------------------------------------------
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] sk0x50 commented on a change in pull request #7613:
IGNITE-12821 Add check size into validate_indexes
Posted by GitBox <gi...@apache.org>.
sk0x50 commented on a change in pull request #7613: IGNITE-12821 Add check size into validate_indexes
URL: https://github.com/apache/ignite/pull/7613#discussion_r407262539
##########
File path: modules/indexing/src/main/java/org/apache/ignite/internal/visor/verify/ValidateIndexesClosure.java
##########
@@ -412,7 +476,7 @@ private IndexIntegrityCheckIssue integrityCheckIndexPartition(CacheGroupContext
finally {
integrityCheckedIndexes.incrementAndGet();
- printProgressIfNeeded("Current progress of ValidateIndexesClosure: checked integrity of "
+ printProgressIfNeeded(() -> "Current progress of ValidateIndexesClosure: checked integrity of "
Review comment:
Is creating a lambda expression less expensive than volatile read? I suppose the right answer is yes. Is it 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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [ignite] sk0x50 commented on a change in pull request #7613:
IGNITE-12821 Add check size into validate_indexes
Posted by GitBox <gi...@apache.org>.
sk0x50 commented on a change in pull request #7613: IGNITE-12821 Add check size into validate_indexes
URL: https://github.com/apache/ignite/pull/7613#discussion_r407597947
##########
File path: modules/core/src/main/java/org/apache/ignite/internal/processors/query/GridQueryProcessor.java
##########
@@ -2906,13 +2908,11 @@ public void remove(GridCacheContext cctx, CacheDataRow row)
* @return Descriptors.
*/
public Collection<GridQueryTypeDescriptor> types(@Nullable String cacheName) {
- Collection<GridQueryTypeDescriptor> cacheTypes = new ArrayList<>();
+ Collection<GridQueryTypeDescriptor> cacheTypes = newSetFromMap(new IdentityHashMap<>());
Review comment:
Ok, got 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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [ignite] tkalkirill commented on a change in pull request #7613:
IGNITE-12821 Add check size into validate_indexes
Posted by GitBox <gi...@apache.org>.
tkalkirill commented on a change in pull request #7613: IGNITE-12821 Add check size into validate_indexes
URL: https://github.com/apache/ignite/pull/7613#discussion_r407587656
##########
File path: modules/indexing/src/main/java/org/apache/ignite/internal/visor/verify/ValidateIndexesClosure.java
##########
@@ -206,53 +230,55 @@ private VisorValidateIndexesJobResult call0() {
List<Future<Map<PartitionKey, ValidateIndexesPartitionResult>>> procPartFutures = new ArrayList<>();
List<Future<Map<String, ValidateIndexesPartitionResult>>> procIdxFutures = new ArrayList<>();
+ List<T3<CacheGroupContext, GridDhtLocalPartition, Future<CacheSize>>> cacheSizeFutures = new ArrayList<>();
+ List<T3<GridCacheContext, Index, Future<T2<Throwable, Long>>>> idxSizeFutures = new ArrayList<>();
+
List<T2<CacheGroupContext, GridDhtLocalPartition>> partArgs = new ArrayList<>();
List<T2<GridCacheContext, Index>> idxArgs = new ArrayList<>();
totalCacheGrps = grpIds.size();
Map<Integer, IndexIntegrityCheckIssue> integrityCheckResults = integrityCheckIndexesPartitions(grpIds);
+ GridQueryProcessor qryProcessor = ignite.context().query();
+ IgniteH2Indexing h2Indexing = (IgniteH2Indexing)qryProcessor.getIndexing();
+
for (Integer grpId : grpIds) {
CacheGroupContext grpCtx = ignite.context().cache().cacheGroup(grpId);
- if (grpCtx == null || integrityCheckResults.containsKey(grpId))
+ if (isNull(grpCtx) || integrityCheckResults.containsKey(grpId))
Review comment:
I know all about it, but it's my style, besides java will eventually remove it, there's no overhead in 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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [ignite] sk0x50 commented on a change in pull request #7613:
IGNITE-12821 Add check size into validate_indexes
Posted by GitBox <gi...@apache.org>.
sk0x50 commented on a change in pull request #7613: IGNITE-12821 Add check size into validate_indexes
URL: https://github.com/apache/ignite/pull/7613#discussion_r407262973
##########
File path: modules/indexing/src/main/java/org/apache/ignite/internal/visor/verify/ValidateIndexesClosure.java
##########
@@ -206,53 +230,55 @@ private VisorValidateIndexesJobResult call0() {
List<Future<Map<PartitionKey, ValidateIndexesPartitionResult>>> procPartFutures = new ArrayList<>();
List<Future<Map<String, ValidateIndexesPartitionResult>>> procIdxFutures = new ArrayList<>();
+ List<T3<CacheGroupContext, GridDhtLocalPartition, Future<CacheSize>>> cacheSizeFutures = new ArrayList<>();
+ List<T3<GridCacheContext, Index, Future<T2<Throwable, Long>>>> idxSizeFutures = new ArrayList<>();
Review comment:
Perhaps, it would be nice to have a dedicated class for these tuples. To be honest, the following line looks weird to me.
> cacheSizeFutures.get(curCacheSize).get3().get();
For instance:
```
class PartitionSizeFuture {
public CacheGroupContext groupContext();
public GridLocalPartition partition();
public CacheSize size();
}
```
----------------------------------------------------------------
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] sk0x50 commented on a change in pull request #7613:
IGNITE-12821 Add check size into validate_indexes
Posted by GitBox <gi...@apache.org>.
sk0x50 commented on a change in pull request #7613: IGNITE-12821 Add check size into validate_indexes
URL: https://github.com/apache/ignite/pull/7613#discussion_r407257271
##########
File path: modules/core/src/main/java/org/apache/ignite/internal/visor/verify/VisorValidateIndexesTaskArg.java
##########
@@ -37,17 +39,20 @@
private Set<String> caches;
/** Check first K elements. */
- private int checkFirst;
-
- /** Check CRC */
- private boolean checkCrc;
+ private int checkFirst = -1;
Review comment:
If I am not mistaken, *-1* and *0*, which is the default, are treated in the same way.
----------------------------------------------------------------
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] sk0x50 commented on a change in pull request #7613:
IGNITE-12821 Add check size into validate_indexes
Posted by GitBox <gi...@apache.org>.
sk0x50 commented on a change in pull request #7613: IGNITE-12821 Add check size into validate_indexes
URL: https://github.com/apache/ignite/pull/7613#discussion_r407256231
##########
File path: modules/core/src/main/java/org/apache/ignite/internal/visor/verify/ValidateIndexesCheckSizeIssue.java
##########
@@ -0,0 +1,107 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.ignite.internal.visor.verify;
+
+import java.io.IOException;
+import java.io.ObjectInput;
+import java.io.ObjectOutput;
+import org.apache.ignite.internal.util.tostring.GridToStringExclude;
+import org.apache.ignite.internal.util.typedef.internal.S;
+import org.apache.ignite.internal.visor.VisorDataTransferObject;
+import org.jetbrains.annotations.Nullable;
+
+import static org.apache.ignite.internal.util.IgniteUtils.readString;
+import static org.apache.ignite.internal.util.IgniteUtils.writeString;
+
+/**
+ * Issue when checking size of cache and index.
+ */
+public class ValidateIndexesCheckSizeIssue extends VisorDataTransferObject {
+ /** Serial version uid. */
+ private static final long serialVersionUID = 0L;
+
+ /** Index name. */
+ private String idxName;
+
+ /** Index size. */
+ private long idxSize;
+
+ /** Error. */
+ @GridToStringExclude
+ private Throwable t;
+
+ /**
+ * Default constructor.
+ */
+ public ValidateIndexesCheckSizeIssue() {
+ //Default constructor required for Externalizable.
+ }
+
+ /**
+ * Constructor.
+ *
+ * @param idxName Index name.
+ * @param idxSize Index size.
+ * @param t Error.
+ */
+ public ValidateIndexesCheckSizeIssue(@Nullable String idxName, long idxSize, @Nullable Throwable t) {
+ this.idxName = idxName;
+ this.idxSize = idxSize;
+ this.t = t;
+ }
+
+ /** {@inheritDoc} */
+ @Override protected void writeExternalData(ObjectOutput out) throws IOException {
+ writeString(out, idxName);
+ out.writeLong(idxSize);
+ out.writeObject(t);
+ }
+
+ /** {@inheritDoc} */
+ @Override protected void readExternalData(
+ byte protoVer,
+ ObjectInput in
+ ) throws IOException, ClassNotFoundException {
+ idxName = readString(in);
+ idxSize = in.readLong();
+ t = (Throwable)in.readObject();
+ }
+
+ /**
+ * Return index size.
+ *
+ * @return Index size.
+ */
+ public long idxSize() {
+ return idxSize;
+ }
+
+ /**
+ * Return error.
+ *
+ * @return Error.
+ */
+ public Throwable err() {
+ return t;
+ }
+
+ /** {@inheritDoc} */
+ @Override public String toString() {
+ return S.toString(ValidateIndexesCheckSizeIssue.class, this) + ", " + t.getClass() + ": " + t.getMessage();
Review comment:
This method may throw *NullPointerException* if the *t* is null. Perhaps it should be as follows:
```
return S.toString(ValidateIndexesCheckSizeIssue.class, this, "err", t);
```
----------------------------------------------------------------
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] tkalkirill closed pull request #7613: IGNITE-12821 Add
check size into validate_indexes
Posted by GitBox <gi...@apache.org>.
tkalkirill closed pull request #7613: IGNITE-12821 Add check size into validate_indexes
URL: https://github.com/apache/ignite/pull/7613
----------------------------------------------------------------
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] tkalkirill commented on a change in pull request #7613:
IGNITE-12821 Add check size into validate_indexes
Posted by GitBox <gi...@apache.org>.
tkalkirill commented on a change in pull request #7613: IGNITE-12821 Add check size into validate_indexes
URL: https://github.com/apache/ignite/pull/7613#discussion_r407580357
##########
File path: modules/core/src/test/java/org/apache/ignite/util/GridCommandHandlerAbstractTest.java
##########
@@ -185,18 +188,17 @@ protected boolean idleVerifyRes(Path p) {
if (sslEnabled())
cfg.setSslContextFactory(GridTestUtils.sslFactory());
- DataStorageConfiguration memCfg = new DataStorageConfiguration()
+ DataStorageConfiguration dsCfg = new DataStorageConfiguration()
+ .setWalMode(WALMode.LOG_ONLY)
Review comment:
I'll leave 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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [ignite] tkalkirill commented on a change in pull request #7613:
IGNITE-12821 Add check size into validate_indexes
Posted by GitBox <gi...@apache.org>.
tkalkirill commented on a change in pull request #7613: IGNITE-12821 Add check size into validate_indexes
URL: https://github.com/apache/ignite/pull/7613#discussion_r407584860
##########
File path: modules/core/src/main/java/org/apache/ignite/internal/processors/query/GridQueryProcessor.java
##########
@@ -2906,13 +2908,11 @@ public void remove(GridCacheContext cctx, CacheDataRow row)
* @return Descriptors.
*/
public Collection<GridQueryTypeDescriptor> types(@Nullable String cacheName) {
- Collection<GridQueryTypeDescriptor> cacheTypes = new ArrayList<>();
+ Collection<GridQueryTypeDescriptor> cacheTypes = newSetFromMap(new IdentityHashMap<>());
Review comment:
Just can be duplicates, look how this Map is filled, start from that line
https://github.com/apache/ignite/blob/5a8b0cc4894a99ef4b22b988b9aa30859485b1b9/modules/core/src/main/java/org/apache/ignite/internal/processors/query/GridQueryProcessor.java#L1833
`
types.put(typeId, desc);
if (altTypeId != null)
types.put(altTypeId, desc);
`
----------------------------------------------------------------
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] sk0x50 commented on a change in pull request #7613:
IGNITE-12821 Add check size into validate_indexes
Posted by GitBox <gi...@apache.org>.
sk0x50 commented on a change in pull request #7613: IGNITE-12821 Add check size into validate_indexes
URL: https://github.com/apache/ignite/pull/7613#discussion_r407255037
##########
File path: modules/core/src/main/java/org/apache/ignite/internal/processors/query/GridQueryProcessor.java
##########
@@ -2906,13 +2908,11 @@ public void remove(GridCacheContext cctx, CacheDataRow row)
* @return Descriptors.
*/
public Collection<GridQueryTypeDescriptor> types(@Nullable String cacheName) {
- Collection<GridQueryTypeDescriptor> cacheTypes = new ArrayList<>();
+ Collection<GridQueryTypeDescriptor> cacheTypes = newSetFromMap(new IdentityHashMap<>());
Review comment:
What is the reason for using *IdentityHashMap*? Should this method return duplicates? It looks like the answer is no.
----------------------------------------------------------------
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] tkalkirill commented on a change in pull request #7613:
IGNITE-12821 Add check size into validate_indexes
Posted by GitBox <gi...@apache.org>.
tkalkirill commented on a change in pull request #7613: IGNITE-12821 Add check size into validate_indexes
URL: https://github.com/apache/ignite/pull/7613#discussion_r407586567
##########
File path: modules/core/src/main/java/org/apache/ignite/internal/visor/verify/VisorValidateIndexesTaskArg.java
##########
@@ -37,17 +39,20 @@
private Set<String> caches;
/** Check first K elements. */
- private int checkFirst;
-
- /** Check CRC */
- private boolean checkCrc;
+ private int checkFirst = -1;
Review comment:
I'm just move from there
https://github.com/apache/ignite/blob/6b6d82a3725a7202ff1effdf905e2463b084cbc7/modules/core/src/main/java/org/apache/ignite/internal/visor/verify/VisorValidateIndexesTaskArg.java#L128
----------------------------------------------------------------
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] sk0x50 commented on a change in pull request #7613:
IGNITE-12821 Add check size into validate_indexes
Posted by GitBox <gi...@apache.org>.
sk0x50 commented on a change in pull request #7613: IGNITE-12821 Add check size into validate_indexes
URL: https://github.com/apache/ignite/pull/7613#discussion_r407255480
##########
File path: modules/core/src/main/java/org/apache/ignite/internal/visor/verify/ValidateIndexesCheckSizeIssue.java
##########
@@ -0,0 +1,107 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.ignite.internal.visor.verify;
+
+import java.io.IOException;
+import java.io.ObjectInput;
+import java.io.ObjectOutput;
+import org.apache.ignite.internal.util.tostring.GridToStringExclude;
+import org.apache.ignite.internal.util.typedef.internal.S;
+import org.apache.ignite.internal.visor.VisorDataTransferObject;
+import org.jetbrains.annotations.Nullable;
+
+import static org.apache.ignite.internal.util.IgniteUtils.readString;
+import static org.apache.ignite.internal.util.IgniteUtils.writeString;
+
+/**
+ * Issue when checking size of cache and index.
+ */
+public class ValidateIndexesCheckSizeIssue extends VisorDataTransferObject {
+ /** Serial version uid. */
+ private static final long serialVersionUID = 0L;
+
+ /** Index name. */
+ private String idxName;
+
+ /** Index size. */
+ private long idxSize;
+
+ /** Error. */
+ @GridToStringExclude
+ private Throwable t;
+
+ /**
+ * Default constructor.
+ */
+ public ValidateIndexesCheckSizeIssue() {
+ //Default constructor required for Externalizable.
+ }
+
+ /**
+ * Constructor.
+ *
+ * @param idxName Index name.
+ * @param idxSize Index size.
+ * @param t Error.
+ */
+ public ValidateIndexesCheckSizeIssue(@Nullable String idxName, long idxSize, @Nullable Throwable t) {
+ this.idxName = idxName;
+ this.idxSize = idxSize;
+ this.t = t;
+ }
+
+ /** {@inheritDoc} */
+ @Override protected void writeExternalData(ObjectOutput out) throws IOException {
+ writeString(out, idxName);
+ out.writeLong(idxSize);
+ out.writeObject(t);
+ }
+
+ /** {@inheritDoc} */
+ @Override protected void readExternalData(
+ byte protoVer,
+ ObjectInput in
+ ) throws IOException, ClassNotFoundException {
+ idxName = readString(in);
+ idxSize = in.readLong();
+ t = (Throwable)in.readObject();
+ }
+
+ /**
+ * Return index size.
+ *
+ * @return Index size.
+ */
+ public long idxSize() {
Review comment:
I don't think that there is a reason for using an abbreviated name here and below.
I would prefer the following names: *indexSize()* and *error()*
----------------------------------------------------------------
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] sk0x50 commented on a change in pull request #7613:
IGNITE-12821 Add check size into validate_indexes
Posted by GitBox <gi...@apache.org>.
sk0x50 commented on a change in pull request #7613: IGNITE-12821 Add check size into validate_indexes
URL: https://github.com/apache/ignite/pull/7613#discussion_r407256328
##########
File path: modules/core/src/main/java/org/apache/ignite/internal/visor/verify/ValidateIndexesCheckSizeResult.java
##########
@@ -0,0 +1,101 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.ignite.internal.visor.verify;
+
+import java.io.IOException;
+import java.io.ObjectInput;
+import java.io.ObjectOutput;
+import java.util.Collection;
+import org.apache.ignite.internal.util.tostring.GridToStringExclude;
+import org.apache.ignite.internal.util.typedef.internal.S;
+import org.apache.ignite.internal.visor.VisorDataTransferObject;
+
+import static java.util.Collections.emptyList;
+import static org.apache.ignite.internal.util.IgniteUtils.readCollection;
+import static org.apache.ignite.internal.util.IgniteUtils.writeCollection;
+
+/**
+ * Result of checking size cache and index.
+ */
+public class ValidateIndexesCheckSizeResult extends VisorDataTransferObject {
Review comment:
I think *ValidateIndexesCheckSizeResult* should extend *IgniteDataTransferObject* instead of deprecated *VisorDataTransferObject*.
----------------------------------------------------------------
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] sk0x50 commented on a change in pull request #7613:
IGNITE-12821 Add check size into validate_indexes
Posted by GitBox <gi...@apache.org>.
sk0x50 commented on a change in pull request #7613: IGNITE-12821 Add check size into validate_indexes
URL: https://github.com/apache/ignite/pull/7613#discussion_r407257544
##########
File path: modules/core/src/test/java/org/apache/ignite/util/GridCommandHandlerAbstractTest.java
##########
@@ -185,18 +188,17 @@ protected boolean idleVerifyRes(Path p) {
if (sslEnabled())
cfg.setSslContextFactory(GridTestUtils.sslFactory());
- DataStorageConfiguration memCfg = new DataStorageConfiguration()
+ DataStorageConfiguration dsCfg = new DataStorageConfiguration()
+ .setWalMode(WALMode.LOG_ONLY)
Review comment:
*LOG_ONLY* mode is the default value, so this call can be omitted. Anyway, it's up to you.
----------------------------------------------------------------
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] sk0x50 commented on a change in pull request #7613:
IGNITE-12821 Add check size into validate_indexes
Posted by GitBox <gi...@apache.org>.
sk0x50 commented on a change in pull request #7613: IGNITE-12821 Add check size into validate_indexes
URL: https://github.com/apache/ignite/pull/7613#discussion_r407254474
##########
File path: modules/core/src/main/java/org/apache/ignite/internal/commandline/cache/CacheValidateIndexes.java
##########
@@ -74,43 +79,65 @@
map.put(CHECK_FIRST + " N", "validate only the first N keys");
map.put(CHECK_THROUGH + " K", "validate every Kth key");
map.put(CHECK_CRC.toString(), "check the CRC-sum of pages stored on disk");
-
- usageCache(logger, VALIDATE_INDEXES, description, map,
- optional(CACHES), OP_NODE_ID, optional(or(CHECK_FIRST + " N", CHECK_THROUGH + " K")));
+ map.put(CHECK_SIZES.toString(), "check that index size and cache size are same");
Review comment:
Please fix the type: "cache that index size and cache size are *the* same".
----------------------------------------------------------------
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] sk0x50 commented on a change in pull request #7613:
IGNITE-12821 Add check size into validate_indexes
Posted by GitBox <gi...@apache.org>.
sk0x50 commented on a change in pull request #7613: IGNITE-12821 Add check size into validate_indexes
URL: https://github.com/apache/ignite/pull/7613#discussion_r407255204
##########
File path: modules/core/src/main/java/org/apache/ignite/internal/visor/verify/ValidateIndexesCheckSizeIssue.java
##########
@@ -0,0 +1,107 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.ignite.internal.visor.verify;
+
+import java.io.IOException;
+import java.io.ObjectInput;
+import java.io.ObjectOutput;
+import org.apache.ignite.internal.util.tostring.GridToStringExclude;
+import org.apache.ignite.internal.util.typedef.internal.S;
+import org.apache.ignite.internal.visor.VisorDataTransferObject;
+import org.jetbrains.annotations.Nullable;
+
+import static org.apache.ignite.internal.util.IgniteUtils.readString;
+import static org.apache.ignite.internal.util.IgniteUtils.writeString;
+
+/**
+ * Issue when checking size of cache and index.
+ */
+public class ValidateIndexesCheckSizeIssue extends VisorDataTransferObject {
Review comment:
I think *ValidateIndexesCheckSizeIssue* should extend *IgniteDataTransferObject* instead of deprecated *VisorDataTransferObject*.
----------------------------------------------------------------
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] sk0x50 commented on a change in pull request #7613:
IGNITE-12821 Add check size into validate_indexes
Posted by GitBox <gi...@apache.org>.
sk0x50 commented on a change in pull request #7613: IGNITE-12821 Add check size into validate_indexes
URL: https://github.com/apache/ignite/pull/7613#discussion_r407256671
##########
File path: modules/core/src/main/java/org/apache/ignite/internal/visor/verify/VisorValidateIndexesJobResult.java
##########
@@ -86,41 +100,58 @@ public VisorValidateIndexesJobResult() {
* @return Results of reverse indexes validation from node.
*/
public Map<String, ValidateIndexesPartitionResult> indexResult() {
- return idxRes == null ? Collections.emptyMap() : idxRes;
+ return idxRes == null ? emptyMap() : idxRes;
}
/**
* @return Collection of failed integrity checks.
*/
public Collection<IndexIntegrityCheckIssue> integrityCheckFailures() {
- return integrityCheckFailures == null ? Collections.emptyList() : integrityCheckFailures;
+ return integrityCheckFailures == null ? emptyList() : integrityCheckFailures;
+ }
+
+ /**
+ * Return results of checking size cache and index.
+ *
+ * @return Results of checking size cache and index.
+ */
+ public Map<String, ValidateIndexesCheckSizeResult> checkSizeResult() {
+ return checkSizeRes == null ? emptyMap() : checkSizeRes;
}
/**
* @return {@code true} If any indexes issues found on node, otherwise returns {@code false}.
*/
public boolean hasIssues() {
return (integrityCheckFailures != null && !integrityCheckFailures.isEmpty()) ||
- (partRes != null && partRes.entrySet().stream().anyMatch(e -> !e.getValue().issues().isEmpty())) ||
- (idxRes != null && idxRes.entrySet().stream().anyMatch(e -> !e.getValue().issues().isEmpty()));
+ (partRes != null && partRes.entrySet().stream().anyMatch(e -> !e.getValue().issues().isEmpty())) ||
+ (idxRes != null && idxRes.entrySet().stream().anyMatch(e -> !e.getValue().issues().isEmpty())) ||
+ (checkSizeRes != null && checkSizeRes.entrySet().stream().anyMatch(e -> !e.getValue().issues().isEmpty()));
}
/** {@inheritDoc} */
@Override protected void writeExternalData(ObjectOutput out) throws IOException {
- U.writeMap(out, partRes);
- U.writeMap(out, idxRes);
- U.writeCollection(out, integrityCheckFailures);
+ writeMap(out, partRes);
+ writeMap(out, idxRes);
+ writeCollection(out, integrityCheckFailures);
+ writeMap(out, checkSizeRes);
}
/** {@inheritDoc} */
- @Override protected void readExternalData(byte protoVer, ObjectInput in) throws IOException, ClassNotFoundException {
- partRes = U.readMap(in);
+ @Override protected void readExternalData(
+ byte protoVer,
+ ObjectInput in
+ ) throws IOException, ClassNotFoundException {
+ partRes = readMap(in);
if (protoVer >= V2)
- idxRes = U.readMap(in);
+ idxRes = readMap(in);
if (protoVer >= V3)
- integrityCheckFailures = U.readCollection(in);
+ integrityCheckFailures = readCollection(in);
+
+ if (protoVer >= V4)
Review comment:
Well, Apache Ignite does not support the rolling upgrade feature, so there is no reason for *V4* and so on.
----------------------------------------------------------------
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] sk0x50 commented on a change in pull request #7613:
IGNITE-12821 Add check size into validate_indexes
Posted by GitBox <gi...@apache.org>.
sk0x50 commented on a change in pull request #7613: IGNITE-12821 Add check size into validate_indexes
URL: https://github.com/apache/ignite/pull/7613#discussion_r407258210
##########
File path: modules/core/src/test/resources/org.apache.ignite.util/GridCommandHandlerClusterByClassTest_cache_help.output
##########
@@ -29,13 +29,14 @@ Arguments: --cache help --yes
--groups - print information about groups.
--seq - print information about sequences.
- --cache validate_indexes [cacheName1,...,cacheNameN] [nodeId] [--check-first N|--check-through K]
+ --cache validate_indexes [cacheName1,...,cacheNameN] [nodeId] [--check-first N|--check-through K|--check-crc|--check-sizes]
Verify counters and hash sums of primary and backup partitions for the specified caches/cache groups on an idle cluster and print out the differences, if any. Cache filtering options configure the set of caches that will be processed by idle_verify command. Default value for the set of cache names (or cache group names) is all cache groups. Default value for --exclude-caches is empty set. Default value for --cache-filter is no filtering. Therefore, the set of all caches is sequently filtered by cache name regexps, by cache type and after all by exclude regexps.
Parameters:
--check-first N - validate only the first N keys
--check-through K - validate every Kth key
--check-crc - check the CRC-sum of pages stored on disk
+ --check-sizes - check that index size and cache size are same
Review comment:
*check that index size and cache size are the same*
----------------------------------------------------------------
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