lhotari commented on code in PR #25548:
URL: https://github.com/apache/pulsar/pull/25548#discussion_r3101426081
##########
managed-ledger/src/main/java/org/apache/bookkeeper/mledger/impl/OpenTelemetryManagedCursorStats.java:
##########
@@ -55,6 +56,16 @@ public class OpenTelemetryManagedCursorStats implements
AutoCloseable {
public static final String INCOMING_BYTE_COUNTER =
"pulsar.broker.managed_ledger.cursor.incoming.size";
private final ObservableLongMeasurement incomingByteCounter;
+ // Broker-level counters incremented when cursor persistence silently
truncates ack state.
+ // See managedLedgerMaxUnackedRangesToPersist and
managedLedgerMaxBatchDeletedIndexToPersist.
+ public static final String PERSIST_OVERFLOW_RANGES_COUNTER =
+ "pulsar.broker.managed_ledger.cursor.persist.overflow.range.count";
Review Comment:
```suggestion
public static final String PERSIST_UNACKED_RANGES_TRUNCATED =
"pulsar.broker.managed_ledger.cursor.persist.unacked_ranges.truncated";
```
Changing the concept from "overflow" to persisting unacked ranges being
truncated would improve the clarity of the counter.
##########
managed-ledger/src/main/java/org/apache/bookkeeper/mledger/impl/OpenTelemetryManagedCursorStats.java:
##########
@@ -55,6 +56,16 @@ public class OpenTelemetryManagedCursorStats implements
AutoCloseable {
public static final String INCOMING_BYTE_COUNTER =
"pulsar.broker.managed_ledger.cursor.incoming.size";
private final ObservableLongMeasurement incomingByteCounter;
+ // Broker-level counters incremented when cursor persistence silently
truncates ack state.
+ // See managedLedgerMaxUnackedRangesToPersist and
managedLedgerMaxBatchDeletedIndexToPersist.
+ public static final String PERSIST_OVERFLOW_RANGES_COUNTER =
+ "pulsar.broker.managed_ledger.cursor.persist.overflow.range.count";
+ private final LongCounter persistOverflowRangesCounter;
Review Comment:
make the field name match the "truncated" concept.
##########
managed-ledger/src/main/java/org/apache/bookkeeper/mledger/impl/ManagedCursorImpl.java:
##########
@@ -3388,11 +3398,32 @@ private List<MessageRange>
buildIndividualDeletedMessageRanges() {
acksSerializedSize.addAndGet(messageRange.getSerializedSize());
rangeList.add(messageRange);
- return rangeList.size() <=
getConfig().getMaxUnackedRangesToPersist();
+ return true;
});
this.individualDeletedMessagesSerializedSize =
acksSerializedSize.get();
individualDeletedMessages.resetDirtyKeys();
+
+ if (overflowed.booleanValue()) {
+
ledger.getFactory().getOpenTelemetryManagedCursorStats().incrementPersistOverflowRanges();
+ if (lastCursorDataFullyPersistable.compareAndSet(true, false))
{
+ int totalRanges = individualDeletedMessages.size();
+ log.warn()
Review Comment:
add attributes for the managed ledger name and cursor name. "managedLedger"
and "cursor" as attribute names.
##########
managed-ledger/src/main/java/org/apache/bookkeeper/mledger/impl/OpenTelemetryManagedCursorStats.java:
##########
@@ -55,6 +56,16 @@ public class OpenTelemetryManagedCursorStats implements
AutoCloseable {
public static final String INCOMING_BYTE_COUNTER =
"pulsar.broker.managed_ledger.cursor.incoming.size";
private final ObservableLongMeasurement incomingByteCounter;
+ // Broker-level counters incremented when cursor persistence silently
truncates ack state.
+ // See managedLedgerMaxUnackedRangesToPersist and
managedLedgerMaxBatchDeletedIndexToPersist.
+ public static final String PERSIST_OVERFLOW_RANGES_COUNTER =
+ "pulsar.broker.managed_ledger.cursor.persist.overflow.range.count";
+ private final LongCounter persistOverflowRangesCounter;
+
+ public static final String PERSIST_OVERFLOW_BATCH_INDEXES_COUNTER =
+
"pulsar.broker.managed_ledger.cursor.persist.overflow.batch.index.count";
Review Comment:
```suggestion
public static final String PERSIST_BATCH_DELETED_INDEXES_TRUNCATED =
"pulsar.broker.managed_ledger.cursor.persist.batch_deleted_indexes.truncated";
```
Similar motivation for the change here, improving the clarity.
##########
managed-ledger/src/main/java/org/apache/bookkeeper/mledger/impl/OpenTelemetryManagedCursorStats.java:
##########
@@ -96,6 +107,22 @@ public OpenTelemetryManagedCursorStats(OpenTelemetry
openTelemetry, ManagedLedge
.setDescription("The total amount of data read from the
ledger.")
.buildObserver();
+ persistOverflowRangesCounter = meter
+ .counterBuilder(PERSIST_OVERFLOW_RANGES_COUNTER)
+ .setUnit("{overflow}")
Review Comment:
```suggestion
.setUnit("{truncation}")
```
##########
managed-ledger/src/main/java/org/apache/bookkeeper/mledger/impl/OpenTelemetryManagedCursorStats.java:
##########
@@ -96,6 +107,22 @@ public OpenTelemetryManagedCursorStats(OpenTelemetry
openTelemetry, ManagedLedge
.setDescription("The total amount of data read from the
ledger.")
.buildObserver();
+ persistOverflowRangesCounter = meter
+ .counterBuilder(PERSIST_OVERFLOW_RANGES_COUNTER)
+ .setUnit("{overflow}")
+ .setDescription("The number of times a cursor exceeded"
+ + " managedLedgerMaxUnackedRangesToPersist, causing
ack state to be truncated"
+ + " at persistence. Ack state beyond the limit is lost
on broker restart.")
+ .build();
+
+ persistOverflowBatchIndexesCounter = meter
+ .counterBuilder(PERSIST_OVERFLOW_BATCH_INDEXES_COUNTER)
+ .setUnit("{overflow}")
Review Comment:
```suggestion
.setUnit("{truncation}")
```
##########
managed-ledger/src/main/java/org/apache/bookkeeper/mledger/impl/ManagedCursorImpl.java:
##########
@@ -3419,6 +3451,26 @@ private List<BatchedEntryDeletionIndexInfo>
buildBatchEntryDeletionIndexInfoList
}
result.add(batchDeletedIndexInfo);
}
+
+ if (iterator.hasNext()) {
+
ledger.getFactory().getOpenTelemetryManagedCursorStats().incrementPersistOverflowBatchIndexes();
+ if (lastBatchDeletedIndexFullyPersistable.compareAndSet(true,
false)) {
+ int totalIndexes = batchDeletedIndexes.size();
+ log.warn()
+ .attr("totalIndexes", totalIndexes)
Review Comment:
add attributes for the managed ledger name and cursor name. "managedLedger"
and "cursor" as attribute names.
--
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]
