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 2021/09/22 16:05:33 UTC

[GitHub] [ignite] anton-vinogradov commented on a change in pull request #9398: IGNITE-14355 CDC Metrics

anton-vinogradov commented on a change in pull request #9398:
URL: https://github.com/apache/ignite/pull/9398#discussion_r714041436



##########
File path: modules/core/src/main/java/org/apache/ignite/internal/cdc/CdcMain.java
##########
@@ -114,12 +115,48 @@
     /** State dir. */
     public static final String STATE_DIR = "state";
 
+    /** Current segment index metric name. */
+    public static final String CUR_SEG_IDX = "CurrentSegmentIndex";
+
+    /** Committed segment index metric name. */
+    public static final String COMMITTED_SEG_IDX = "CommittedSegmentIndex";
+
+    /** Committed segment offset metric name. */
+    public static final String COMMITTED_SEG_OFF = "CommittedSegmentOffset";
+
+    /** Last segment consumption time. */
+    public static final String LAST_SEG_CONSUMPTION_TIME = "LastSegmentConsumptionTime";
+
+    /** Binary metadata metric name. */
+    public static final String BINARY_META = "BinaryMeta";

Review comment:
       DIR & Dir?

##########
File path: modules/core/src/main/java/org/apache/ignite/internal/cdc/CdcMain.java
##########
@@ -114,12 +115,39 @@
     /** State dir. */
     public static final String STATE_DIR = "state";
 
+    /** Current segment index metric name. */
+    public static final String CUR_SEG_IDX = "CurrentSegmentIndex";
+
+    /** Committed segment index metric name. */
+    public static final String COMMITTED_SEG_IDX = "CommittedSegmentIndex";
+
+    /** Committed segment offset metric name. */
+    public static final String COMMITTED_SEG_OFF = "CommittedSegmentOffset";

Review comment:
       `OFFSET`, OFF means disable(d)

##########
File path: modules/core/src/test/java/org/apache/ignite/cdc/AbstractCdcTest.java
##########
@@ -111,16 +132,69 @@ public boolean waitForSize(
             timeout);
     }
 
+    /** */
+    public long checkMetrics(CdcMain cdc, int expCnt) throws Exception {

Review comment:
       expCnt never checked

##########
File path: modules/core/src/main/java/org/apache/ignite/internal/cdc/WalRecordsConsumer.java
##########
@@ -123,6 +137,10 @@ private boolean hasCurrent() {
 
             /** */
             private CdcEvent transform(DataEntry e) {
+                evtsCnt.increment();
+
+                lastEvtTs.value(System.currentTimeMillis());

Review comment:
       It looks like the `next()` method is a better place for counting and ts recording.

##########
File path: modules/core/src/main/java/org/apache/ignite/internal/cdc/CdcMain.java
##########
@@ -114,12 +115,48 @@
     /** State dir. */
     public static final String STATE_DIR = "state";
 
+    /** Current segment index metric name. */
+    public static final String CUR_SEG_IDX = "CurrentSegmentIndex";
+
+    /** Committed segment index metric name. */
+    public static final String COMMITTED_SEG_IDX = "CommittedSegmentIndex";
+
+    /** Committed segment offset metric name. */
+    public static final String COMMITTED_SEG_OFF = "CommittedSegmentOffset";
+
+    /** Last segment consumption time. */
+    public static final String LAST_SEG_CONSUMPTION_TIME = "LastSegmentConsumptionTime";
+
+    /** Binary metadata metric name. */
+    public static final String BINARY_META = "BinaryMeta";
+
+    /** Marshaller metric name. */
+    public static final String MARSHALLER = "Marshaller";

Review comment:
       DIR & Dir?

##########
File path: modules/core/src/main/java/org/apache/ignite/spi/metric/jmx/JmxMetricExporterSpi.java
##########
@@ -132,7 +132,7 @@ else if (log.isDebugEnabled())
 
             ObjectName mbean = U.registerMBean(
                 ignite().configuration().getMBeanServer(),
-                igniteInstanceName,
+                igniteInstanceName == null ? ignite().configuration().getIgniteInstanceName() : igniteInstanceName,

Review comment:
       Why no just make `igniteInstanceName == ignite().configuration().getIgniteInstanceName()` instead of `if`?
   Even another question:
   why igniteInstanceName != ignite().configuration().getIgniteInstanceName()`?




-- 
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