alievmirza commented on code in PR #7910:
URL: https://github.com/apache/ignite-3/pull/7910#discussion_r3038730815
##########
modules/core/src/main/java/org/apache/ignite/internal/util/OffheapReadWriteLock.java:
##########
@@ -25,20 +25,25 @@
import java.util.concurrent.locks.Condition;
import java.util.concurrent.locks.ReentrantLock;
import org.apache.ignite.internal.lang.IgniteSystemProperties;
+import org.apache.ignite.internal.logger.IgniteLogger;
+import org.apache.ignite.internal.logger.Loggers;
+import org.apache.ignite.internal.thread.ThreadUtils;
import org.apache.ignite.internal.tostring.S;
import org.jetbrains.annotations.Nullable;
/**
* Lock state structure is as follows.
* <pre>
- * +----------------+---------------+---------+----------+
- * | WRITE WAIT CNT | READ WAIT CNT | TAG | LOCK CNT |
- * +----------------+---------------+---------+----------+
- * | 2 bytes | 2 bytes | 2 bytes | 2 bytes |
- * +----------------+---------------+---------+----------+
+ * +----------------+---------------+---------+----------+----------+
Review Comment:
Seems that `VolatilePageMemory` uses this lock, but it's javadoc is not
fixed and lock is still 8 bytes. Please check all affected javadocs (if
presented) and fix.
##########
modules/core/src/main/java/org/apache/ignite/internal/util/OffheapReadWriteLock.java:
##########
@@ -326,6 +346,9 @@ public void writeUnlock(long lock, int tag) {
+ "[lock=" + hexLong(lock) + ", state=" +
hexLong(state) + ']');
}
+ // Do it strictly after the check, that the best we can do in the
absence of K-CAS.
+ clearOwnerId(lock);
Review Comment:
and it would be nice to have a bit more detailed comment, I do not fully
understand it
##########
modules/core/src/main/java/org/apache/ignite/internal/thread/ThreadUtils.java:
##########
@@ -53,6 +53,38 @@ public class ThreadUtils {
/** System line separator. */
private static final String NL = System.lineSeparator();
+ /**
+ * Performs thread dump and prints all available info to the given log
with {@code WARN} or {@code ERROR} logging level depending on
+ * {@code isErrorLevel} parameter. If there's no thread with a given ID,
or ID is invalid, then nothing is printed.
+ *
+ * @param log Logger.
+ * @param threadId ID of a thread to dump.
+ * @param isErrorLevel {@code true} if thread dump must be printed with
{@code ERROR} logging level, {@code false} if thread dump must
+ * be printed with {@code WARN} logging level.
+ */
+ public static void dumpThread(IgniteLogger log, long threadId, boolean
isErrorLevel) {
+ if (threadId <= 0) {
+ return;
+ }
+
+ // We don't really need a full stack, and shorter trace should be less
disruptive I think.
+ int maxStackElements = 15;
Review Comment:
Does this really impact performance? I mean getting 15 elements instead of
full stack? On the one hand is precise info that could help with debugging, on
the other possible perf impact that is not proved.
##########
modules/core/src/main/java/org/apache/ignite/internal/util/OffheapReadWriteLock.java:
##########
@@ -326,6 +346,9 @@ public void writeUnlock(long lock, int tag) {
+ "[lock=" + hexLong(lock) + ", state=" +
hexLong(state) + ']');
}
+ // Do it strictly after the check, that the best we can do in the
absence of K-CAS.
+ clearOwnerId(lock);
Review Comment:
comment about comparing local threadId and ownerId and throwing exception
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
