This is an automated email from the ASF dual-hosted git repository.
timoninmaxim pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/ignite.git
The following commit(s) were added to refs/heads/master by this push:
new 6b67d839cdd IGNITE-24777 Fix reading performance statistics report due
to miss-cached strings (#11949)
6b67d839cdd is described below
commit 6b67d839cdd54299d938b4317fa8c027925a5d44
Author: Aleksandr Chesnokov <[email protected]>
AuthorDate: Tue Mar 25 14:52:19 2025 +0300
IGNITE-24777 Fix reading performance statistics report due to miss-cached
strings (#11949)
Thank you for submitting the pull request to the Apache Ignite.
In order to streamline the review of the contribution
we ask you to ensure the following steps have been taken:
### The Contribution Checklist
- [ ] There is a single JIRA ticket related to the pull request.
- [x] The web-link to the pull request is attached to the JIRA ticket.
- [ ] The JIRA ticket has the _Patch Available_ state.
- [x] The pull request body describes changes that have been made.
The description explains _WHAT_ and _WHY_ was made instead of _HOW_.
- [x] The pull request title is treated as the final commit message.
The following pattern must be used: `IGNITE-XXXX Change summary` where
`XXXX` - number of JIRA issue.
- [ ] A reviewer has been mentioned through the JIRA comments
(see [the Maintainers
list](https://cwiki.apache.org/confluence/display/IGNITE/How+to+Contribute#HowtoContribute-ReviewProcessandMaintainers))
- [x] The pull request has been checked by the Teamcity Bot and
the `green visa` attached to the JIRA ticket (see [TC.Bot: Check
PR](https://mtcga.gridgain.com/prs.html))
### Notes
- [How to
Contribute](https://cwiki.apache.org/confluence/display/IGNITE/How+to+Contribute)
- [Coding abbreviation
rules](https://cwiki.apache.org/confluence/display/IGNITE/Abbreviation+Rules)
- [Coding
Guidelines](https://cwiki.apache.org/confluence/display/IGNITE/Coding+Guidelines)
- [Apache Ignite Teamcity
Bot](https://cwiki.apache.org/confluence/display/IGNITE/Apache+Ignite+Teamcity+Bot)
If you need any help, please email [email protected] or ask anу
advice on http://asf.slack.com _#ignite_ channel.
---
.../FilePerformanceStatisticsReader.java | 209 ++++++++-------------
.../FilePerformanceStatisticsWriter.java | 2 +-
.../performancestatistics/OperationType.java | 36 ++++
.../ForwardReadQueryPropertyTest.java | 103 ++++++++++
.../IgniteBasicWithPersistenceTestSuite.java | 2 +
5 files changed, 221 insertions(+), 131 deletions(-)
diff --git
a/modules/core/src/main/java/org/apache/ignite/internal/processors/performancestatistics/FilePerformanceStatisticsReader.java
b/modules/core/src/main/java/org/apache/ignite/internal/processors/performancestatistics/FilePerformanceStatisticsReader.java
index e902dfc7db3..6084c24901b 100644
---
a/modules/core/src/main/java/org/apache/ignite/internal/processors/performancestatistics/FilePerformanceStatisticsReader.java
+++
b/modules/core/src/main/java/org/apache/ignite/internal/processors/performancestatistics/FilePerformanceStatisticsReader.java
@@ -60,13 +60,15 @@ import static
org.apache.ignite.internal.processors.performancestatistics.Operat
import static
org.apache.ignite.internal.processors.performancestatistics.OperationType.VERSION;
import static
org.apache.ignite.internal.processors.performancestatistics.OperationType.cacheOperation;
import static
org.apache.ignite.internal.processors.performancestatistics.OperationType.cacheRecordSize;
-import static
org.apache.ignite.internal.processors.performancestatistics.OperationType.cacheStartRecordSize;
import static
org.apache.ignite.internal.processors.performancestatistics.OperationType.checkpointRecordSize;
import static
org.apache.ignite.internal.processors.performancestatistics.OperationType.jobRecordSize;
import static
org.apache.ignite.internal.processors.performancestatistics.OperationType.pagesWriteThrottleRecordSize;
import static
org.apache.ignite.internal.processors.performancestatistics.OperationType.queryReadsRecordSize;
-import static
org.apache.ignite.internal.processors.performancestatistics.OperationType.queryRecordSize;
-import static
org.apache.ignite.internal.processors.performancestatistics.OperationType.taskRecordSize;
+import static
org.apache.ignite.internal.processors.performancestatistics.OperationType.readCacheStartRecordSize;
+import static
org.apache.ignite.internal.processors.performancestatistics.OperationType.readQueryPropertyRecordSize;
+import static
org.apache.ignite.internal.processors.performancestatistics.OperationType.readQueryRecordSize;
+import static
org.apache.ignite.internal.processors.performancestatistics.OperationType.readQueryRowsRecordSize;
+import static
org.apache.ignite.internal.processors.performancestatistics.OperationType.readTaskRecordSize;
import static
org.apache.ignite.internal.processors.performancestatistics.OperationType.transactionOperation;
import static
org.apache.ignite.internal.processors.performancestatistics.OperationType.transactionRecordSize;
@@ -267,36 +269,10 @@ public class FilePerformanceStatisticsReader {
return true;
}
else if (opType == QUERY) {
- if (buf.remaining() < 1)
- return false;
-
- boolean cached = buf.get() != 0;
-
- String text;
- int hash = 0;
-
- if (cached) {
- if (buf.remaining() < 4)
- return false;
-
- hash = buf.getInt();
-
- text = knownStrs.get(hash);
-
- if (buf.remaining() < queryRecordSize(0, true) - 1 - 4)
- return false;
- }
- else {
- if (buf.remaining() < 4)
- return false;
-
- int textLen = buf.getInt();
-
- if (buf.remaining() < queryRecordSize(textLen, false) - 1 - 4)
- return false;
+ ForwardableString text = readString(buf);
- text = readString(buf, textLen);
- }
+ if (text == null || buf.remaining() < readQueryRecordSize())
+ return false;
GridCacheQueryType qryType =
GridCacheQueryType.fromOrdinal(buf.get());
long id = buf.getLong();
@@ -304,11 +280,10 @@ public class FilePerformanceStatisticsReader {
long duration = buf.getLong();
boolean success = buf.get() != 0;
- if (text == null)
- forwardRead(hash);
+ forwardRead(text);
for (PerformanceStatisticsHandler hnd : curHnd)
- hnd.query(nodeId, qryType, text, id, startTime, duration,
success);
+ hnd.query(nodeId, qryType, text.str, id, startTime, duration,
success);
return true;
}
@@ -328,9 +303,9 @@ public class FilePerformanceStatisticsReader {
return true;
}
else if (opType == QUERY_ROWS) {
- String action = readCacheableString(buf);
+ ForwardableString action = readString(buf);
- if (action == null || buf.remaining() < 1 + 16 + 8 + 8)
+ if (action == null || buf.remaining() < readQueryRowsRecordSize())
return false;
GridCacheQueryType qryType =
GridCacheQueryType.fromOrdinal(buf.get());
@@ -338,76 +313,54 @@ public class FilePerformanceStatisticsReader {
long id = buf.getLong();
long rows = buf.getLong();
+ forwardRead(action);
+
for (PerformanceStatisticsHandler hnd : curHnd)
- hnd.queryRows(nodeId, qryType, uuid, id, action, rows);
+ hnd.queryRows(nodeId, qryType, uuid, id, action.str, rows);
return true;
}
else if (opType == QUERY_PROPERTY) {
- String name = readCacheableString(buf);
+ ForwardableString name = readString(buf);
if (name == null)
return false;
- String val = readCacheableString(buf);
+ ForwardableString val = readString(buf);
if (val == null)
return false;
- if (buf.remaining() < 1 + 16 + 8)
+ if (buf.remaining() < readQueryPropertyRecordSize())
return false;
GridCacheQueryType qryType =
GridCacheQueryType.fromOrdinal(buf.get());
UUID uuid = readUuid(buf);
long id = buf.getLong();
+ forwardRead(name);
+ forwardRead(val);
+
for (PerformanceStatisticsHandler hnd : curHnd)
- hnd.queryProperty(nodeId, qryType, uuid, id, name, val);
+ hnd.queryProperty(nodeId, qryType, uuid, id, name.str,
val.str);
return true;
}
else if (opType == TASK) {
- if (buf.remaining() < 1)
- return false;
-
- boolean cached = buf.get() != 0;
-
- String taskName;
- int hash = 0;
-
- if (cached) {
- if (buf.remaining() < 4)
- return false;
-
- hash = buf.getInt();
+ ForwardableString taskName = readString(buf);
- taskName = knownStrs.get(hash);
-
- if (buf.remaining() < taskRecordSize(0, true) - 1 - 4)
- return false;
- }
- else {
- if (buf.remaining() < 4)
- return false;
-
- int nameLen = buf.getInt();
-
- if (buf.remaining() < taskRecordSize(nameLen, false) - 1 - 4)
- return false;
-
- taskName = readString(buf, nameLen);
- }
+ if (taskName == null || buf.remaining() < readTaskRecordSize())
+ return false;
IgniteUuid sesId = readIgniteUuid(buf);
long startTime = buf.getLong();
long duration = buf.getLong();
int affPartId = buf.getInt();
- if (taskName == null)
- forwardRead(hash);
+ forwardRead(taskName);
for (PerformanceStatisticsHandler hnd : curHnd)
- hnd.task(nodeId, sesId, taskName, startTime, duration,
affPartId);
+ hnd.task(nodeId, sesId, taskName.str, startTime, duration,
affPartId);
return true;
}
@@ -427,41 +380,17 @@ public class FilePerformanceStatisticsReader {
return true;
}
else if (opType == CACHE_START) {
- if (buf.remaining() < 1)
- return false;
-
- boolean cached = buf.get() != 0;
+ ForwardableString cacheName = readString(buf);
- String cacheName;
- int hash = 0;
-
- if (cached) {
- if (buf.remaining() < 4)
- return false;
-
- hash = buf.getInt();
-
- cacheName = knownStrs.get(hash);
-
- if (buf.remaining() < cacheStartRecordSize(0, true) - 1 - 4)
- return false;
- }
- else {
- if (buf.remaining() < 4)
- return false;
-
- int nameLen = buf.getInt();
-
- if (buf.remaining() < cacheStartRecordSize(nameLen, false) - 1
- 4)
- return false;
-
- cacheName = readString(buf, nameLen);
- }
+ if (cacheName == null || buf.remaining() <
readCacheStartRecordSize())
+ return false;
int cacheId = buf.getInt();
+ forwardRead(cacheName);
+
for (PerformanceStatisticsHandler hnd : curHnd)
- hnd.cacheStart(nodeId, cacheId, cacheName);
+ hnd.cacheStart(nodeId, cacheId, cacheName.str);
return true;
}
@@ -524,8 +453,14 @@ public class FilePerformanceStatisticsReader {
throw new IgniteException("Unknown operation type id [typeId=" +
opTypeByte + ']');
}
- /** Turns on forward read mode. */
- private void forwardRead(int hash) throws IOException {
+ /**
+ * Enables forward read mode when {@link ForwardableString#str} is null.
+ * @see ForwardableString
+ */
+ private void forwardRead(ForwardableString forwardableStr) throws
IOException {
+ if (forwardableStr.str != null)
+ return;
+
if (forwardRead != null)
return;
@@ -543,7 +478,7 @@ public class FilePerformanceStatisticsReader {
curHnd = NOOP_HANDLER;
- forwardRead = new ForwardRead(hash, curRecPos, nextRecPos, bufPos);
+ forwardRead = new ForwardRead(forwardableStr.hash, curRecPos,
nextRecPos, bufPos);
}
/** Resolves performance statistics files. */
@@ -585,9 +520,31 @@ public class FilePerformanceStatisticsReader {
return null;
}
- /** Reads string from byte buffer. */
- private String readString(ByteBuffer buf, int size) {
- byte[] bytes = new byte[size];
+ /**
+ * Reads cacheable string from byte buffer.
+ *
+ * @return {@link ForwardableString} with result of reading or {@code
null} in case of buffer underflow.
+ */
+ private ForwardableString readString(ByteBuffer buf) {
+ if (buf.remaining() < 1 + 4)
+ return null;
+
+ boolean cached = buf.get() != 0;
+
+ if (cached) {
+ int hash = buf.getInt();
+
+ String str = knownStrs.get(hash);
+
+ return new ForwardableString(str, hash);
+ }
+
+ int textLen = buf.getInt();
+
+ if (buf.remaining() < textLen)
+ return null;
+
+ byte[] bytes = new byte[textLen];
buf.get(bytes);
@@ -598,32 +555,24 @@ public class FilePerformanceStatisticsReader {
if (forwardRead != null && forwardRead.hash == str.hashCode())
forwardRead.found = true;
- return str;
+ return new ForwardableString(str, str.hashCode());
}
/**
- * Reads cacheable string from byte buffer.
- *
- * @return String or {@code null} in case of buffer underflow.
+ * Result of reading string from buffer that may be cached.
+ * Call {@link #forwardRead(ForwardableString)} after reading the entire
record to enable forward read mode.
*/
- private String readCacheableString(ByteBuffer buf) {
- if (buf.remaining() < 1 + 4)
- return null;
+ private static class ForwardableString {
+ /** Can be {@code null} if the string is cached and there is no such
{@link #hash} in {@link #knownStrs}. */
+ @Nullable final String str;
- boolean cached = buf.get() != 0;
-
- if (cached) {
- int hash = buf.getInt();
-
- return knownStrs.get(hash);
- }
- else {
- int textLen = buf.getInt();
-
- if (buf.remaining() < textLen)
- return null;
+ /** */
+ final int hash;
- return readString(buf, textLen);
+ /** */
+ ForwardableString(@Nullable String str, int hash) {
+ this.str = str;
+ this.hash = hash;
}
}
diff --git
a/modules/core/src/main/java/org/apache/ignite/internal/processors/performancestatistics/FilePerformanceStatisticsWriter.java
b/modules/core/src/main/java/org/apache/ignite/internal/processors/performancestatistics/FilePerformanceStatisticsWriter.java
index 9907dce1025..7d7495bd6ec 100644
---
a/modules/core/src/main/java/org/apache/ignite/internal/processors/performancestatistics/FilePerformanceStatisticsWriter.java
+++
b/modules/core/src/main/java/org/apache/ignite/internal/processors/performancestatistics/FilePerformanceStatisticsWriter.java
@@ -511,7 +511,7 @@ public class FilePerformanceStatisticsWriter {
}
/** Writes {@link UUID} to buffer. */
- private static void writeUuid(ByteBuffer buf, UUID uuid) {
+ static void writeUuid(ByteBuffer buf, UUID uuid) {
buf.putLong(uuid.getMostSignificantBits());
buf.putLong(uuid.getLeastSignificantBits());
}
diff --git
a/modules/core/src/main/java/org/apache/ignite/internal/processors/performancestatistics/OperationType.java
b/modules/core/src/main/java/org/apache/ignite/internal/processors/performancestatistics/OperationType.java
index 131cd9c84a8..e1fb8f20485 100644
---
a/modules/core/src/main/java/org/apache/ignite/internal/processors/performancestatistics/OperationType.java
+++
b/modules/core/src/main/java/org/apache/ignite/internal/processors/performancestatistics/OperationType.java
@@ -163,6 +163,13 @@ public enum OperationType {
return 1 + 4 + (cached ? 4 : 4 + nameLen);
}
+ /**
+ * @return Cache start record size left after reading name string.
+ */
+ public static int readCacheStartRecordSize() {
+ return cacheStartRecordSize(0, true) - 1 /* cached flag */ - 4 /* hash
or len of str */;
+ }
+
/** @return Cache record size. */
public static int cacheRecordSize() {
return 4 + 8 + 8;
@@ -185,6 +192,13 @@ public enum OperationType {
return 1 + (cached ? 4 : 4 + textLen) + 1 + 8 + 8 + 8 + 1;
}
+ /**
+ * @return Query record size left after reading text string.
+ */
+ public static int readQueryRecordSize() {
+ return queryRecordSize(0, true) - 1 /* cached flag */ - 4 /* hash or
len of str */;
+ }
+
/** @return Query reads record size. */
public static int queryReadsRecordSize() {
return 1 + 16 + 8 + 8 + 8;
@@ -199,6 +213,13 @@ public enum OperationType {
return 1 + (cached ? 4 : 4 + actionLen) + 1 + 16 + 8 + 8;
}
+ /**
+ * @return Query rows record size left after reading action string.
+ */
+ public static int readQueryRowsRecordSize() {
+ return queryRowsRecordSize(0, true) - 1 /* cached flag */ - 4 /* hash
or len of str */;
+ }
+
/**
* @param nameLen Propery name length.
* @param nameCached {@code True} if property name is cached.
@@ -210,6 +231,14 @@ public enum OperationType {
return 1 + (nameCached ? 4 : 4 + nameLen) + 1 + (valCached ? 4 : 4 +
valLen) + 1 + 16 + 8;
}
+ /**
+ * @return Query property record size left after reading name and val
strings.
+ */
+ public static int readQueryPropertyRecordSize() {
+ return queryPropertyRecordSize(0, true, 0, true)
+ - 1 /* cached flag */ - 4 /* hash or len of name str */ - 1 /*
cached flag */ - 4 /* hash or len of val str */;
+ }
+
/**
* @param nameLen Task name length.
* @param cached {@code True} if task name cached.
@@ -219,6 +248,13 @@ public enum OperationType {
return 1 + (cached ? 4 : 4 + nameLen) + 24 + 8 + 8 + 4;
}
+ /**
+ * @return Task record size left after reading name string.
+ */
+ public static int readTaskRecordSize() {
+ return taskRecordSize(0, true) - 1 /* cached flag */ - 4 /* hash or
len of str */;
+ }
+
/** @return Job record size. */
public static int jobRecordSize() {
return 24 + 8 + 8 + 8 + 1;
diff --git
a/modules/core/src/test/java/org/apache/ignite/internal/processors/performancestatistics/ForwardReadQueryPropertyTest.java
b/modules/core/src/test/java/org/apache/ignite/internal/processors/performancestatistics/ForwardReadQueryPropertyTest.java
new file mode 100644
index 00000000000..1fe7f1e6eb2
--- /dev/null
+++
b/modules/core/src/test/java/org/apache/ignite/internal/processors/performancestatistics/ForwardReadQueryPropertyTest.java
@@ -0,0 +1,103 @@
+/*
+ * 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.processors.performancestatistics;
+
+import java.io.File;
+import java.nio.ByteBuffer;
+import java.nio.ByteOrder;
+import java.util.HashMap;
+import java.util.Map;
+import java.util.UUID;
+import org.apache.ignite.internal.processors.cache.persistence.file.FileIO;
+import
org.apache.ignite.internal.processors.cache.persistence.file.RandomAccessFileIOFactory;
+import org.apache.ignite.internal.processors.cache.query.GridCacheQueryType;
+import org.apache.ignite.internal.util.typedef.internal.U;
+import org.junit.Test;
+
+import static java.util.Collections.singletonList;
+import static java.util.UUID.randomUUID;
+import static
org.apache.ignite.internal.processors.performancestatistics.FilePerformanceStatisticsWriter.PERF_STAT_DIR;
+import static
org.apache.ignite.internal.processors.performancestatistics.FilePerformanceStatisticsWriter.writeString;
+import static
org.apache.ignite.internal.processors.performancestatistics.FilePerformanceStatisticsWriter.writeUuid;
+
+/**
+ * Tests forward read mode for {@link OperationType#QUERY_PROPERTY} records.
+ */
+public class ForwardReadQueryPropertyTest extends
AbstractPerformanceStatisticsTest {
+ /** Read buffer size. */
+ private static final int BUFFER_SIZE = 100;
+
+ /** @throws Exception If failed. */
+ @Test
+ public void testStringForwardRead() throws Exception {
+ File dir = U.resolveWorkDirectory(U.defaultWorkDirectory(),
PERF_STAT_DIR, false);
+
+ Map<String, String> expProps = createStatistics(dir);
+ Map<String, String> actualProps = new HashMap<>();
+
+ new FilePerformanceStatisticsReader(BUFFER_SIZE, new TestHandler() {
+ @Override public void queryProperty(UUID nodeId,
GridCacheQueryType type, UUID qryNodeId, long id, String name,
+ String val) {
+ assertNotNull(name);
+ assertNotNull(val);
+
+ actualProps.put(name, val);
+ }
+ }).read(singletonList(dir));
+
+ assertEquals(expProps, actualProps);
+ }
+
+ /** Creates test performance statistics file. */
+ private Map<String, String> createStatistics(File dir) throws Exception {
+ File file = new File(dir, "node-" + randomUUID() + ".prf");
+
+ try (FileIO fileIo = new RandomAccessFileIOFactory().create(file)) {
+ ByteBuffer buf =
ByteBuffer.allocate(1024).order(ByteOrder.nativeOrder());
+
+ buf.put(OperationType.VERSION.id());
+ buf.putShort(FilePerformanceStatisticsWriter.FILE_FORMAT_VERSION);
+
+ writeQueryProperty(buf, "property", true, "val", true);
+ writeQueryProperty(buf, "property", false, "val", false);
+ writeQueryProperty(buf, "propertyNan", true, "valNan", true);
+
+ buf.flip();
+
+ fileIo.write(buf);
+
+ fileIo.force();
+ }
+
+ return Map.of("property", "val");
+ }
+
+ /** */
+ private static void writeQueryProperty(ByteBuffer buf, String name,
boolean nameCached, String val, boolean valCached) {
+ buf.put(OperationType.QUERY_PROPERTY.id());
+
+ writeString(buf, name, nameCached);
+ writeString(buf, val, valCached);
+
+ buf.put((byte)GridCacheQueryType.SQL_FIELDS.ordinal());
+
+ writeUuid(buf, randomUUID());
+
+ buf.putLong(0);
+ }
+}
diff --git
a/modules/core/src/test/java/org/apache/ignite/testsuites/IgniteBasicWithPersistenceTestSuite.java
b/modules/core/src/test/java/org/apache/ignite/testsuites/IgniteBasicWithPersistenceTestSuite.java
index 721c5adb40d..840844ff6f5 100644
---
a/modules/core/src/test/java/org/apache/ignite/testsuites/IgniteBasicWithPersistenceTestSuite.java
+++
b/modules/core/src/test/java/org/apache/ignite/testsuites/IgniteBasicWithPersistenceTestSuite.java
@@ -42,6 +42,7 @@ import
org.apache.ignite.internal.processors.cache.persistence.CommonPoolStarvat
import
org.apache.ignite.internal.processors.cache.persistence.SingleNodePersistenceSslTest;
import
org.apache.ignite.internal.processors.performancestatistics.CacheStartTest;
import
org.apache.ignite.internal.processors.performancestatistics.CheckpointTest;
+import
org.apache.ignite.internal.processors.performancestatistics.ForwardReadQueryPropertyTest;
import
org.apache.ignite.internal.processors.performancestatistics.ForwardReadTest;
import
org.apache.ignite.internal.processors.performancestatistics.PerformanceStatisticsMultipleStartTest;
import
org.apache.ignite.internal.processors.performancestatistics.PerformanceStatisticsPropertiesTest;
@@ -103,6 +104,7 @@ import org.junit.runners.Suite;
PerformanceStatisticsPropertiesTest.class,
PerformanceStatisticsMultipleStartTest.class,
ForwardReadTest.class,
+ ForwardReadQueryPropertyTest.class,
CacheStartTest.class,
CheckpointTest.class
})
