From 6050f572c124100083cf3341487df257983f852a Mon Sep 17 00:00:00 2001 From: KirillKurdyukov Date: Fri, 9 Oct 2026 15:24:19 +0300 Subject: [PATCH 1/9] Add Topic session error counters with settings-owned Writer metrics --- .../ydb/topic/impl/TopicMetricsUtils.java | 21 +++++++++++++++ .../tech/ydb/topic/read/impl/ReaderImpl.java | 2 ++ .../ydb/topic/read/impl/ReaderMetrics.java | 9 +++++++ .../tech/ydb/topic/write/impl/WriterImpl.java | 6 +++++ .../ydb/topic/write/impl/WriterMetrics.java | 9 +++++++ .../topic/read/impl/ReaderMetricsTest.java | 23 ++++++++++++++++ .../topic/write/impl/WriterMetricsTest.java | 26 +++++++++++++++++++ 7 files changed, 96 insertions(+) create mode 100644 topic/src/main/java/tech/ydb/topic/impl/TopicMetricsUtils.java diff --git a/topic/src/main/java/tech/ydb/topic/impl/TopicMetricsUtils.java b/topic/src/main/java/tech/ydb/topic/impl/TopicMetricsUtils.java new file mode 100644 index 000000000..81a4ee585 --- /dev/null +++ b/topic/src/main/java/tech/ydb/topic/impl/TopicMetricsUtils.java @@ -0,0 +1,21 @@ +package tech.ydb.topic.impl; + +import java.util.Arrays; + +import tech.ydb.core.StatusCode; +import tech.ydb.core.metrics.Attr; + +public final class TopicMetricsUtils { + private TopicMetricsUtils() { + } + + public static Attr[] sessionErrorAttributes(Attr[] commonAttributes, StatusCode code, boolean retry) { + Attr[] attributes = Arrays.copyOf(commonAttributes, commonAttributes.length + 3); + attributes[commonAttributes.length] = Attr.of("retry_decision", retry ? "retry" : "stop"); + attributes[commonAttributes.length + 1] = Attr.of("status_code", code.name()); + String errorType = code == StatusCode.SUCCESS || code == StatusCode.UNUSED_STATUS + ? "session_closed" : code.isTransportError() ? "transport_error" : "ydb_error"; + attributes[commonAttributes.length + 2] = Attr.of("error.type", errorType); + return attributes; + } +} diff --git a/topic/src/main/java/tech/ydb/topic/read/impl/ReaderImpl.java b/topic/src/main/java/tech/ydb/topic/read/impl/ReaderImpl.java index 13c27d643..9f41b933c 100644 --- a/topic/src/main/java/tech/ydb/topic/read/impl/ReaderImpl.java +++ b/topic/src/main/java/tech/ydb/topic/read/impl/ReaderImpl.java @@ -67,6 +67,7 @@ protected CompletableFuture> createNewStream(String id) { @Override protected void onRetry(ReadSession stream, Status status) { + config.getMetrics().reportSessionError(status, true); logger.warn("[{}] stopped by status {}", debugId, status); currentSessionId = null; if (errorHandler != null) { @@ -85,6 +86,7 @@ protected void onRetry(ReadSession stream, Status status) { protected void onClose(ReadSession stream, Status status) { currentSessionId = null; if (!status.isSuccess()) { + config.getMetrics().reportSessionError(status, false); logger.warn("[{}] closed by status {}", debugId, status); } else { logger.info("[{}] closed by status {}", debugId, status); diff --git a/topic/src/main/java/tech/ydb/topic/read/impl/ReaderMetrics.java b/topic/src/main/java/tech/ydb/topic/read/impl/ReaderMetrics.java index d2f6efdd6..209e09187 100644 --- a/topic/src/main/java/tech/ydb/topic/read/impl/ReaderMetrics.java +++ b/topic/src/main/java/tech/ydb/topic/read/impl/ReaderMetrics.java @@ -3,10 +3,12 @@ import java.util.Arrays; import java.util.function.LongSupplier; +import tech.ydb.core.Status; import tech.ydb.core.metrics.Attr; import tech.ydb.core.metrics.LongCounter; import tech.ydb.core.metrics.Meter; import tech.ydb.core.metrics.MetricRegistration; +import tech.ydb.topic.impl.TopicMetricsUtils; /** * Topic reader metrics. @@ -17,6 +19,7 @@ final class ReaderMetrics { private final LongCounter deliveredMessages; private final LongCounter receivedMessages; private final LongCounter receivedBytes; + private final LongCounter sessionErrors; private final Attr[] commonAttributes; private final boolean enabled; private final Meter meter; @@ -34,6 +37,8 @@ final class ReaderMetrics { "The number of messages accepted by the SDK for an active partition session."); this.receivedBytes = meter.createCounter("ydb.topic.reader.received.bytes", "By", "The protocol bytes_size received in read responses."); + this.sessionErrors = meter.createCounter("ydb.topic.reader.session.errors", "{error}", + "The number of reader stream session errors by retry decision."); this.commonAttributes = createCommonAttributes(consumer, readerName); } @@ -68,6 +73,10 @@ void reportReceivedBytes(long bytes) { } } + void reportSessionError(Status status, boolean retry) { + sessionErrors.add(1, TopicMetricsUtils.sessionErrorAttributes(commonAttributes, status.getCode(), retry)); + } + private void report(LongCounter counter, long messages, String topic) { if (enabled && messages > 0) { record(counter, messages, Attr.of("topic", topic)); diff --git a/topic/src/main/java/tech/ydb/topic/write/impl/WriterImpl.java b/topic/src/main/java/tech/ydb/topic/write/impl/WriterImpl.java index 72289751e..5b694c443 100644 --- a/topic/src/main/java/tech/ydb/topic/write/impl/WriterImpl.java +++ b/topic/src/main/java/tech/ydb/topic/write/impl/WriterImpl.java @@ -39,6 +39,7 @@ public class WriterImpl { private final String debugId; private final WriterQueue writeQueue; + private final WriterMetrics metrics; private final WriteSession stream; private final Runnable sendTask = new SerialRunnable(new SendTask()); @@ -59,6 +60,7 @@ public WriterImpl(TopicRpc topicRpc, WriteStreamFactory factory, WriterSettings this.debugId = DebugTools.createDebugId(settings.getLogPrefix()); this.stream = new WriteSession(debugId, factory, settings, topicRpc.getScheduler(), new ListenerImpl()); this.writeQueue = new WriterQueue(debugId, settings, codecRegistry, compressionExecutor, sendTask); + this.metrics = new WriterMetrics(settings.getMeter(), settings.getTopicPath(), settings.getWriterName()); Observability.reportMetricsUsage(settings.getMeter()); logger.info("Writer with id {} created for topic \"{}\" with producerId \"{}\" and messageGroupId \"{}\"", @@ -185,6 +187,7 @@ public void onStart(long lastSeqNo, String sessionId) { @Override public void onStop(Status status) { isReady = false; + metrics.reportSessionError(status, true); } @Override @@ -194,6 +197,9 @@ public void onAck(WriteAck ack) { @Override public void onClose(Status status) { + if (!isClosed.get()) { + metrics.reportSessionError(status, false); + } isClosed.set(true); isReady = false; initFuture.completeExceptionally(new UnexpectedResultException("Cannot init write session", status)); diff --git a/topic/src/main/java/tech/ydb/topic/write/impl/WriterMetrics.java b/topic/src/main/java/tech/ydb/topic/write/impl/WriterMetrics.java index 3cf097710..735610d0b 100644 --- a/topic/src/main/java/tech/ydb/topic/write/impl/WriterMetrics.java +++ b/topic/src/main/java/tech/ydb/topic/write/impl/WriterMetrics.java @@ -2,10 +2,12 @@ import java.util.function.LongSupplier; +import tech.ydb.core.Status; import tech.ydb.core.metrics.Attr; import tech.ydb.core.metrics.LongCounter; import tech.ydb.core.metrics.Meter; import tech.ydb.core.metrics.MetricRegistration; +import tech.ydb.topic.impl.TopicMetricsUtils; /** * Topic writer metrics. @@ -16,6 +18,7 @@ final class WriterMetrics { private final LongCounter sendingMessages; private final LongCounter sendingBytes; private final LongCounter writtenMessages; + private final LongCounter sessionErrors; private final Attr[] commonAttributes; private final Meter meter; private MetricRegistration bufferUsedGauge = MetricRegistration.NOOP; @@ -29,6 +32,8 @@ final class WriterMetrics { "The uncompressed body size of messages accepted by the writer."); this.writtenMessages = meter.createCounter("ydb.topic.writer.written.messages", MESSAGE_UNIT, "The number of messages confirmed written by the server, including already written messages."); + this.sessionErrors = meter.createCounter("ydb.topic.writer.session.errors", "{error}", + "The number of writer stream session errors by retry decision."); this.commonAttributes = writerName == null ? new Attr[0] : new Attr[]{Attr.of("topic", topic), Attr.of("writer.name", writerName)}; @@ -58,4 +63,8 @@ void reportSending(long bytes) { void reportWritten() { writtenMessages.add(1, commonAttributes); } + + void reportSessionError(Status status, boolean retry) { + sessionErrors.add(1, TopicMetricsUtils.sessionErrorAttributes(commonAttributes, status.getCode(), retry)); + } } diff --git a/topic/src/test/java/tech/ydb/topic/read/impl/ReaderMetricsTest.java b/topic/src/test/java/tech/ydb/topic/read/impl/ReaderMetricsTest.java index c300e764d..e9d39497a 100644 --- a/topic/src/test/java/tech/ydb/topic/read/impl/ReaderMetricsTest.java +++ b/topic/src/test/java/tech/ydb/topic/read/impl/ReaderMetricsTest.java @@ -31,6 +31,28 @@ public class ReaderMetricsTest { + @Test + public void sessionErrorsIncrementOnStreamFailure() { + RecordingMeter meter = new RecordingMeter(); + ReadStreamMock stream = new ReadStreamMock(); + TopicRpc rpc = Mockito.mock(TopicRpc.class); + Mockito.when(rpc.getScheduler()).thenReturn(Mockito.mock(ScheduledExecutorService.class)); + Mockito.when(rpc.readSession(Mockito.anyString())).thenReturn(stream); + TopicClient client = TopicClientImpl.newClient(rpc).build(); + SyncReader reader = client.createSyncReader(ReaderSettings.newBuilder() + .addTopic("/topic").setConsumerName("consumer").setRetryConfig(status -> null) + .withMeter(meter, "reader").build()); + try { + reader.init(); + stream.responseInit("session"); + stream.closeStream(Status.of(StatusCode.UNAUTHORIZED)); + Assert.assertEquals(1, meter.value("ydb.topic.reader.session.errors")); + } finally { + reader.shutdown(); + client.close(); + } + } + @Test public void readerCountersIncrementOnReceive() throws InterruptedException { String delivered = "ydb.topic.reader.delivered.messages"; @@ -79,6 +101,7 @@ public void readerCountersIncrementOnReceive() throws InterruptedException { reader.shutdown(); client.close(); } + Assert.assertEquals(0, meter.value("ydb.topic.reader.session.errors")); } @Test diff --git a/topic/src/test/java/tech/ydb/topic/write/impl/WriterMetricsTest.java b/topic/src/test/java/tech/ydb/topic/write/impl/WriterMetricsTest.java index 1a4e7b03b..6fd32e29f 100644 --- a/topic/src/test/java/tech/ydb/topic/write/impl/WriterMetricsTest.java +++ b/topic/src/test/java/tech/ydb/topic/write/impl/WriterMetricsTest.java @@ -11,6 +11,7 @@ import org.mockito.Mockito; import tech.ydb.core.Status; +import tech.ydb.core.StatusCode; import tech.ydb.core.metrics.LongCounter; import tech.ydb.core.metrics.LongMeasurement; import tech.ydb.core.metrics.Meter; @@ -26,6 +27,31 @@ public class WriterMetricsTest { + @Test + public void sessionErrorsIncrementOnStreamFailure() { + Map counters = new HashMap<>(); + Meter meter = new Meter() { + @Override + public LongCounter createCounter(String name, String unit, String description) { + return (value, attrs) -> counters.merge(name, value, Long::sum); + } + }; + WriteStreamMock stream = new WriteStreamMock(); + WriterSettings settings = WriterSettings.newBuilder() + .setTopicPath("/test/topic").setCodec(Codec.RAW).setRetryConfig(status -> null) + .withMeter(meter, "writer").build(); + try (TopicClient client = TopicClientImpl.newClient(mockRpc(stream)).build()) { + AsyncWriter writer = client.createAsyncWriter(settings); + try { + writer.init(); + stream.close(Status.of(StatusCode.UNAUTHORIZED)); + Assert.assertEquals(Long.valueOf(1), counters.get("ydb.topic.writer.session.errors")); + } finally { + writer.shutdown(); + } + } + } + @Test public void writerCountersCountAcceptedMessagesAndAcknowledgements() throws QueueOverflowException { Map counters = new HashMap<>(); From a16300dcefa489ac2a21329b199fbf139224a158 Mon Sep 17 00:00:00 2001 From: KirillKurdyukov Date: Fri, 9 Oct 2026 15:33:49 +0300 Subject: [PATCH 2/9] Share Writer metrics between writer and queue --- .../tech/ydb/topic/write/impl/WriterImpl.java | 4 +-- .../ydb/topic/write/impl/WriterQueue.java | 4 +-- .../topic/write/impl/WriterMetricsTest.java | 2 +- .../ydb/topic/write/impl/WriterQueueTest.java | 28 +++++++++++-------- 4 files changed, 22 insertions(+), 16 deletions(-) diff --git a/topic/src/main/java/tech/ydb/topic/write/impl/WriterImpl.java b/topic/src/main/java/tech/ydb/topic/write/impl/WriterImpl.java index 5b694c443..bf7e118a2 100644 --- a/topic/src/main/java/tech/ydb/topic/write/impl/WriterImpl.java +++ b/topic/src/main/java/tech/ydb/topic/write/impl/WriterImpl.java @@ -58,9 +58,9 @@ public WriterImpl(TopicRpc topicRpc, WriterSettings settings, Executor compressi public WriterImpl(TopicRpc topicRpc, WriteStreamFactory factory, WriterSettings settings, Executor compressionExecutor, @Nonnull CodecRegistry codecRegistry) { this.debugId = DebugTools.createDebugId(settings.getLogPrefix()); - this.stream = new WriteSession(debugId, factory, settings, topicRpc.getScheduler(), new ListenerImpl()); - this.writeQueue = new WriterQueue(debugId, settings, codecRegistry, compressionExecutor, sendTask); this.metrics = new WriterMetrics(settings.getMeter(), settings.getTopicPath(), settings.getWriterName()); + this.stream = new WriteSession(debugId, factory, settings, topicRpc.getScheduler(), new ListenerImpl()); + this.writeQueue = new WriterQueue(debugId, settings, metrics, codecRegistry, compressionExecutor, sendTask); Observability.reportMetricsUsage(settings.getMeter()); logger.info("Writer with id {} created for topic \"{}\" with producerId \"{}\" and messageGroupId \"{}\"", diff --git a/topic/src/main/java/tech/ydb/topic/write/impl/WriterQueue.java b/topic/src/main/java/tech/ydb/topic/write/impl/WriterQueue.java index 709725373..6cd2a823f 100644 --- a/topic/src/main/java/tech/ydb/topic/write/impl/WriterQueue.java +++ b/topic/src/main/java/tech/ydb/topic/write/impl/WriterQueue.java @@ -59,7 +59,7 @@ interface EncodedMsg { // Future for flush method. Only the future is stored to not retain the message data after sending private volatile CompletableFuture lastAcceptedAckFuture = null; - public WriterQueue(String debugId, WriterSettings settings, CodecRegistry codecRegistry, + public WriterQueue(String debugId, WriterSettings settings, WriterMetrics metrics, CodecRegistry codecRegistry, Executor compressionExecutor, Runnable readyNotify) { this.debugId = debugId; this.buffer = new BufferManager(debugId, settings); @@ -70,7 +70,7 @@ public WriterQueue(String debugId, WriterSettings settings, CodecRegistry codecR } this.compressionExecutor = compressionExecutor; this.readyNotify = readyNotify; - this.metrics = new WriterMetrics(settings.getMeter(), settings.getTopicPath(), settings.getWriterName()); + this.metrics = metrics; this.metrics.register(buffer::getUsedSize, buffer::getMaxSize); } diff --git a/topic/src/test/java/tech/ydb/topic/write/impl/WriterMetricsTest.java b/topic/src/test/java/tech/ydb/topic/write/impl/WriterMetricsTest.java index 6fd32e29f..63589ecdf 100644 --- a/topic/src/test/java/tech/ydb/topic/write/impl/WriterMetricsTest.java +++ b/topic/src/test/java/tech/ydb/topic/write/impl/WriterMetricsTest.java @@ -58,7 +58,7 @@ public void writerCountersCountAcceptedMessagesAndAcknowledgements() throws Queu Meter meter = new Meter() { @Override public LongCounter createCounter(String name, String unit, String description) { - counters.put(name, 0L); + Assert.assertNull(counters.put(name, 0L)); return (value, attrs) -> counters.put(name, counters.get(name) + value); } }; diff --git a/topic/src/test/java/tech/ydb/topic/write/impl/WriterQueueTest.java b/topic/src/test/java/tech/ydb/topic/write/impl/WriterQueueTest.java index 8a067ba71..d4bacf56e 100644 --- a/topic/src/test/java/tech/ydb/topic/write/impl/WriterQueueTest.java +++ b/topic/src/test/java/tech/ydb/topic/write/impl/WriterQueueTest.java @@ -19,6 +19,7 @@ import tech.ydb.core.Status; import tech.ydb.core.StatusCode; +import tech.ydb.core.metrics.Meter; import tech.ydb.topic.description.Codec; import tech.ydb.topic.description.CodecRegistry; import tech.ydb.topic.settings.WriterSettings; @@ -58,10 +59,14 @@ private static WriterSettings gzipSettings() { } private static WriterQueue rawQueue(AtomicInteger notifyCount) { - return new WriterQueue("test", rawSettings(), new CodecRegistry(), + return new WriterQueue("test", rawSettings(), metrics(), new CodecRegistry(), Runnable::run, () -> notifyCount.incrementAndGet()); } + private static WriterMetrics metrics() { + return new WriterMetrics(Meter.NOOP, "/test", null); + } + private static void assertOverflow(String msg, ThrowingRunnable runnable) { QueueOverflowException ex = Assert.assertThrows("Must be thrown QueueOverflowException", QueueOverflowException.class, runnable); @@ -88,7 +93,7 @@ public void testInvalidCodecThrows() { .setCodec(9999) .build(); IllegalArgumentException ex = Assert.assertThrows(IllegalArgumentException.class, - () -> new WriterQueue("test", settings, new CodecRegistry(), Runnable::run, () -> { })); + () -> new WriterQueue("test", settings, metrics(), new CodecRegistry(), Runnable::run, () -> { })); Assert.assertEquals("Unsupported codec: 9999", ex.getMessage()); } @@ -102,7 +107,7 @@ public void testFlushWithNoMessages() { @Test public void testRawCompressor() throws Exception { - WriterQueue q = new WriterQueue("test", rawSettings(), new CodecRegistry(), null, () -> {}); + WriterQueue q = new WriterQueue("test", rawSettings(), metrics(), new CodecRegistry(), null, () -> {}); CompletableFuture f1 = q.enqueue(SMALL_MSG, null); CompletableFuture f2 = q.tryEnqueue(SMALL_MSG, null); @@ -133,7 +138,7 @@ public void testCompressedExpansionIsAccounted() throws Exception { .setCodec(codec.getId()) .setMaxSendBufferMemorySize(10) .build(); - WriterQueue writerQueue = new WriterQueue("test", settings, new CodecRegistry(singletonList(codec)), + WriterQueue writerQueue = new WriterQueue("test", settings, metrics(), new CodecRegistry(singletonList(codec)), Runnable::run, () -> { }); @@ -153,7 +158,8 @@ public void testLzopFramingIsAccounted() throws Exception { .setCodec(Codec.LZOP) .setMaxSendBufferMemorySize(109) .build(); - WriterQueue writerQueue = new WriterQueue("test", settings, new CodecRegistry(), encodingTasks::add, () -> { + WriterQueue writerQueue = new WriterQueue("test", settings, metrics(), new CodecRegistry(), encodingTasks::add, + () -> { }); writerQueue.tryEnqueue(SMALL_MSG, null); @@ -183,7 +189,7 @@ public void testEncodedMessageLargerThanBufferIsProcessed() throws Exception { .setCodec(codec.getId()) .setMaxSendBufferMemorySize(10) .build(); - WriterQueue writerQueue = new WriterQueue("test", settings, new CodecRegistry(singletonList(codec)), + WriterQueue writerQueue = new WriterQueue("test", settings, metrics(), new CodecRegistry(singletonList(codec)), encodingTasks::add, () -> { }); @@ -214,7 +220,7 @@ public void testExpansionUsesAvailableCapacity() throws Exception { .setCodec(codec.getId()) .setMaxSendBufferMemorySize(12) .build(); - WriterQueue writerQueue = new WriterQueue("test", settings, new CodecRegistry(singletonList(codec)), + WriterQueue writerQueue = new WriterQueue("test", settings, metrics(), new CodecRegistry(singletonList(codec)), encodingTasks::add, () -> { }); @@ -238,7 +244,7 @@ public void testExpansionUsesAvailableCapacity() throws Exception { @Test @HideLoggers({ WriterImpl.class }) public void testGzipNullCompressor() throws Exception { - WriterQueue q = new WriterQueue("test", gzipSettings(), new CodecRegistry(), null, () -> {}); + WriterQueue q = new WriterQueue("test", gzipSettings(), metrics(), new CodecRegistry(), null, () -> {}); CompletableFuture f1 = q.enqueue(SMALL_MSG, null); CompletableFuture f2 = q.tryEnqueue(SMALL_MSG, null); @@ -283,7 +289,7 @@ public OutputStream encode(OutputStream out) throws IOException { .setCodec(failingCodec.getId()) .build(); - WriterQueue q = new WriterQueue("test", settings, registry, Runnable::run, notify::incrementAndGet); + WriterQueue q = new WriterQueue("test", settings, metrics(), registry, Runnable::run, notify::incrementAndGet); CompletableFuture f1 = q.enqueue(SMALL_MSG, null); CompletableFuture f2 = q.tryEnqueue(SMALL_MSG, null); @@ -335,7 +341,7 @@ public void write(int b) throws IOException { .build(); AtomicInteger notify = new AtomicInteger(); CodecRegistry registry = new CodecRegistry(Arrays.asList(selective)); - WriterQueue q = new WriterQueue("test", settings, registry, Runnable::run, notify::incrementAndGet); + WriterQueue q = new WriterQueue("test", settings, metrics(), registry, Runnable::run, notify::incrementAndGet); CompletableFuture good1 = q.enqueue(Message.of(new byte[] { 0x01 }), null); CompletableFuture bad = q.enqueue(Message.of(new byte[] { 0x7F }), null); @@ -406,7 +412,7 @@ public void testSmallBufferWriting() throws QueueOverflowException { .setMaxSendBufferMemorySize(12) .build(); - WriterQueue q = new WriterQueue("test", settings, new CodecRegistry(), null, notify::incrementAndGet); + WriterQueue q = new WriterQueue("test", settings, metrics(), new CodecRegistry(), null, notify::incrementAndGet); q.tryEnqueue(smallMsg(10), null); // success q.tryEnqueue(smallMsg(20), null); // success From ee8f475e1f149441107d9b509abd6e0e6382b899 Mon Sep 17 00:00:00 2001 From: KirillKurdyukov Date: Thu, 15 Oct 2026 15:54:07 +0300 Subject: [PATCH 3/9] Address Topic session metrics review feedback --- .../ydb/topic/impl/TopicMetricsUtils.java | 21 -- .../ydb/topic/read/impl/ReaderMetrics.java | 11 +- .../tech/ydb/topic/write/impl/WriterImpl.java | 3 +- .../ydb/topic/write/impl/WriterMetrics.java | 12 +- .../ydb/topic/write/impl/WriterQueue.java | 9 + .../topic/read/impl/ReaderMetricsTest.java | 264 ++++++++---------- .../tech/ydb/topic/utils/TopicMeterMock.java | 67 +++++ .../topic/write/impl/WriterMetricsTest.java | 168 +++++------ .../ydb/topic/write/impl/WriterQueueTest.java | 28 +- 9 files changed, 304 insertions(+), 279 deletions(-) delete mode 100644 topic/src/main/java/tech/ydb/topic/impl/TopicMetricsUtils.java create mode 100644 topic/src/test/java/tech/ydb/topic/utils/TopicMeterMock.java diff --git a/topic/src/main/java/tech/ydb/topic/impl/TopicMetricsUtils.java b/topic/src/main/java/tech/ydb/topic/impl/TopicMetricsUtils.java deleted file mode 100644 index 81a4ee585..000000000 --- a/topic/src/main/java/tech/ydb/topic/impl/TopicMetricsUtils.java +++ /dev/null @@ -1,21 +0,0 @@ -package tech.ydb.topic.impl; - -import java.util.Arrays; - -import tech.ydb.core.StatusCode; -import tech.ydb.core.metrics.Attr; - -public final class TopicMetricsUtils { - private TopicMetricsUtils() { - } - - public static Attr[] sessionErrorAttributes(Attr[] commonAttributes, StatusCode code, boolean retry) { - Attr[] attributes = Arrays.copyOf(commonAttributes, commonAttributes.length + 3); - attributes[commonAttributes.length] = Attr.of("retry_decision", retry ? "retry" : "stop"); - attributes[commonAttributes.length + 1] = Attr.of("status_code", code.name()); - String errorType = code == StatusCode.SUCCESS || code == StatusCode.UNUSED_STATUS - ? "session_closed" : code.isTransportError() ? "transport_error" : "ydb_error"; - attributes[commonAttributes.length + 2] = Attr.of("error.type", errorType); - return attributes; - } -} diff --git a/topic/src/main/java/tech/ydb/topic/read/impl/ReaderMetrics.java b/topic/src/main/java/tech/ydb/topic/read/impl/ReaderMetrics.java index 209e09187..0a8804304 100644 --- a/topic/src/main/java/tech/ydb/topic/read/impl/ReaderMetrics.java +++ b/topic/src/main/java/tech/ydb/topic/read/impl/ReaderMetrics.java @@ -4,11 +4,11 @@ import java.util.function.LongSupplier; import tech.ydb.core.Status; +import tech.ydb.core.StatusCode; import tech.ydb.core.metrics.Attr; import tech.ydb.core.metrics.LongCounter; import tech.ydb.core.metrics.Meter; import tech.ydb.core.metrics.MetricRegistration; -import tech.ydb.topic.impl.TopicMetricsUtils; /** * Topic reader metrics. @@ -74,7 +74,14 @@ void reportReceivedBytes(long bytes) { } void reportSessionError(Status status, boolean retry) { - sessionErrors.add(1, TopicMetricsUtils.sessionErrorAttributes(commonAttributes, status.getCode(), retry)); + StatusCode code = status.getCode(); + Attr[] attributes = Arrays.copyOf(commonAttributes, commonAttributes.length + 3); + attributes[commonAttributes.length] = Attr.of("retry_decision", retry ? "retry" : "stop"); + attributes[commonAttributes.length + 1] = Attr.of("status_code", code.name()); + String errorType = code == StatusCode.SUCCESS || code == StatusCode.UNUSED_STATUS + ? "session_closed" : code.isTransportError() ? "transport_error" : "ydb_error"; + attributes[commonAttributes.length + 2] = Attr.of("error.type", errorType); + sessionErrors.add(1, attributes); } private void report(LongCounter counter, long messages, String topic) { diff --git a/topic/src/main/java/tech/ydb/topic/write/impl/WriterImpl.java b/topic/src/main/java/tech/ydb/topic/write/impl/WriterImpl.java index bf7e118a2..744e98a44 100644 --- a/topic/src/main/java/tech/ydb/topic/write/impl/WriterImpl.java +++ b/topic/src/main/java/tech/ydb/topic/write/impl/WriterImpl.java @@ -197,10 +197,9 @@ public void onAck(WriteAck ack) { @Override public void onClose(Status status) { - if (!isClosed.get()) { + if (isClosed.compareAndSet(false, true)) { metrics.reportSessionError(status, false); } - isClosed.set(true); isReady = false; initFuture.completeExceptionally(new UnexpectedResultException("Cannot init write session", status)); shutdownFuture.complete(status); diff --git a/topic/src/main/java/tech/ydb/topic/write/impl/WriterMetrics.java b/topic/src/main/java/tech/ydb/topic/write/impl/WriterMetrics.java index 735610d0b..3fc70f6da 100644 --- a/topic/src/main/java/tech/ydb/topic/write/impl/WriterMetrics.java +++ b/topic/src/main/java/tech/ydb/topic/write/impl/WriterMetrics.java @@ -1,13 +1,14 @@ package tech.ydb.topic.write.impl; +import java.util.Arrays; import java.util.function.LongSupplier; import tech.ydb.core.Status; +import tech.ydb.core.StatusCode; import tech.ydb.core.metrics.Attr; import tech.ydb.core.metrics.LongCounter; import tech.ydb.core.metrics.Meter; import tech.ydb.core.metrics.MetricRegistration; -import tech.ydb.topic.impl.TopicMetricsUtils; /** * Topic writer metrics. @@ -65,6 +66,13 @@ void reportWritten() { } void reportSessionError(Status status, boolean retry) { - sessionErrors.add(1, TopicMetricsUtils.sessionErrorAttributes(commonAttributes, status.getCode(), retry)); + StatusCode code = status.getCode(); + Attr[] attributes = Arrays.copyOf(commonAttributes, commonAttributes.length + 3); + attributes[commonAttributes.length] = Attr.of("retry_decision", retry ? "retry" : "stop"); + attributes[commonAttributes.length + 1] = Attr.of("status_code", code.name()); + String errorType = code == StatusCode.SUCCESS || code == StatusCode.UNUSED_STATUS + ? "session_closed" : code.isTransportError() ? "transport_error" : "ydb_error"; + attributes[commonAttributes.length + 2] = Attr.of("error.type", errorType); + sessionErrors.add(1, attributes); } } diff --git a/topic/src/main/java/tech/ydb/topic/write/impl/WriterQueue.java b/topic/src/main/java/tech/ydb/topic/write/impl/WriterQueue.java index 6cd2a823f..f08a91034 100644 --- a/topic/src/main/java/tech/ydb/topic/write/impl/WriterQueue.java +++ b/topic/src/main/java/tech/ydb/topic/write/impl/WriterQueue.java @@ -13,6 +13,7 @@ import java.util.concurrent.TimeUnit; import java.util.concurrent.TimeoutException; +import com.google.common.annotations.VisibleForTesting; import com.google.protobuf.ByteString; import com.google.protobuf.UnsafeByteOperations; import org.slf4j.Logger; @@ -20,6 +21,7 @@ import tech.ydb.common.transaction.YdbTransaction; import tech.ydb.core.Status; +import tech.ydb.core.metrics.Meter; import tech.ydb.topic.description.Codec; import tech.ydb.topic.description.CodecRegistry; import tech.ydb.topic.settings.WriterSettings; @@ -59,6 +61,13 @@ interface EncodedMsg { // Future for flush method. Only the future is stored to not retain the message data after sending private volatile CompletableFuture lastAcceptedAckFuture = null; + @VisibleForTesting + public WriterQueue(String debugId, WriterSettings settings, CodecRegistry codecRegistry, + Executor compressionExecutor, Runnable readyNotify) { + this(debugId, settings, new WriterMetrics(Meter.NOOP, settings.getTopicPath(), settings.getWriterName()), + codecRegistry, compressionExecutor, readyNotify); + } + public WriterQueue(String debugId, WriterSettings settings, WriterMetrics metrics, CodecRegistry codecRegistry, Executor compressionExecutor, Runnable readyNotify) { this.debugId = debugId; diff --git a/topic/src/test/java/tech/ydb/topic/read/impl/ReaderMetricsTest.java b/topic/src/test/java/tech/ydb/topic/read/impl/ReaderMetricsTest.java index e9d39497a..61d3a8ac0 100644 --- a/topic/src/test/java/tech/ydb/topic/read/impl/ReaderMetricsTest.java +++ b/topic/src/test/java/tech/ydb/topic/read/impl/ReaderMetricsTest.java @@ -1,14 +1,6 @@ package tech.ydb.topic.read.impl; -import java.util.ArrayList; -import java.util.Arrays; -import java.util.List; -import java.util.Map; -import java.util.concurrent.ConcurrentHashMap; import java.util.concurrent.ScheduledExecutorService; -import java.util.concurrent.TimeUnit; -import java.util.concurrent.atomic.AtomicLong; -import java.util.function.Consumer; import org.junit.Assert; import org.junit.Test; @@ -16,172 +8,134 @@ import tech.ydb.core.Status; import tech.ydb.core.StatusCode; -import tech.ydb.core.metrics.Attr; -import tech.ydb.core.metrics.LongCounter; -import tech.ydb.core.metrics.LongMeasurement; -import tech.ydb.core.metrics.Meter; -import tech.ydb.core.metrics.MetricRegistration; import tech.ydb.topic.TopicClient; import tech.ydb.topic.TopicRpc; import tech.ydb.topic.description.Codec; import tech.ydb.topic.impl.TopicClientImpl; import tech.ydb.topic.read.SyncReader; import tech.ydb.topic.settings.ReaderSettings; -import tech.ydb.topic.settings.TopicReadSettings; +import tech.ydb.topic.utils.TopicMeterMock; public class ReaderMetricsTest { + private static final String DELIVERED = "ydb.topic.reader.delivered.messages"; + private static final String RECEIVED = "ydb.topic.reader.received.messages"; + private static final String BYTES = "ydb.topic.reader.received.bytes"; + private static final String ERRORS = "ydb.topic.reader.session.errors"; + private static final String PARTITIONS = "ydb.topic.reader.partition_session.count"; + private static final String CREDIT = "ydb.topic.reader.credit_balance_bytes"; @Test - public void sessionErrorsIncrementOnStreamFailure() { - RecordingMeter meter = new RecordingMeter(); + public void readerReceivesMessagesAndShutsDown() throws InterruptedException { + TopicMeterMock meter = new TopicMeterMock(); ReadStreamMock stream = new ReadStreamMock(); - TopicRpc rpc = Mockito.mock(TopicRpc.class); - Mockito.when(rpc.getScheduler()).thenReturn(Mockito.mock(ScheduledExecutorService.class)); - Mockito.when(rpc.readSession(Mockito.anyString())).thenReturn(stream); - TopicClient client = TopicClientImpl.newClient(rpc).build(); - SyncReader reader = client.createSyncReader(ReaderSettings.newBuilder() - .addTopic("/topic").setConsumerName("consumer").setRetryConfig(status -> null) - .withMeter(meter, "reader").build()); - try { - reader.init(); - stream.responseInit("session"); - stream.closeStream(Status.of(StatusCode.UNAUTHORIZED)); - Assert.assertEquals(1, meter.value("ydb.topic.reader.session.errors")); - } finally { - reader.shutdown(); - client.close(); + try (TopicClient client = TopicClientImpl.newClient(mockRpc(stream)).build()) { + SyncReader reader = client.createSyncReader(ReaderSettings.newBuilder() + .addTopic("/topic").setConsumerName("consumer") + .setMaxMemoryUsageBytes(100).withMeter(meter, "reader").build()); + try { + meter.assertNoGauges(); + reader.init(); + stream.assertLastMessage().isInitRequest("consumer", "/topic").hasReaderName("reader"); + meter.assertNoGauges(); + stream.responseInit("session"); + Assert.assertEquals(0, meter.collect(PARTITIONS)); + Assert.assertEquals(100, meter.collect(CREDIT)); + stream.responseStartPartition("/topic", 42, 0); + Assert.assertEquals(1, meter.collect(PARTITIONS)); + + byte[] msg1 = new byte[]{1}; + byte[] msg2 = new byte[]{2}; + stream.responseData(25).partition(1, 0).batch(Codec.RAW, msg1, msg2).and().send(); + Assert.assertEquals(2, meter.value(RECEIVED)); + Assert.assertEquals(25, meter.value(BYTES)); + Assert.assertEquals(75, meter.collect(CREDIT)); + Assert.assertEquals(0, meter.value(DELIVERED)); + Assert.assertEquals(0, meter.value(ERRORS)); + + stream.responseData(30).partition(1, 2).batch(Codec.RAW, msg1, msg2, msg1).and().send(); + Assert.assertEquals(5, meter.value(RECEIVED)); + Assert.assertEquals(55, meter.value(BYTES)); + Assert.assertEquals(45, meter.collect(CREDIT)); + reader.receive(); + reader.receive(); + Assert.assertEquals(2, meter.value(DELIVERED)); + reader.receive(); + reader.receive(); + reader.receive(); + Assert.assertEquals(5, meter.value(DELIVERED)); + Assert.assertEquals(100, meter.collect(CREDIT)); + Assert.assertEquals(1, meter.collect(PARTITIONS)); + meter.assertAttribute(RECEIVED, "topic", "/topic"); + meter.assertAttribute(DELIVERED, "topic", "/topic"); + meter.assertAttribute(BYTES, "consumer", "consumer"); + meter.assertAttribute(PARTITIONS, "reader.name", "reader"); + meter.assertAttribute(CREDIT, "consumer", "consumer"); + } finally { + reader.shutdown(); + } } + Assert.assertEquals(0, meter.value(ERRORS)); + meter.assertNoGauges(); } @Test - public void readerCountersIncrementOnReceive() throws InterruptedException { - String delivered = "ydb.topic.reader.delivered.messages"; - String receivedMessages = "ydb.topic.reader.received.messages"; - String receivedBytes = "ydb.topic.reader.received.bytes"; - RecordingMeter meter = new RecordingMeter(); - ReadStreamMock stream = new ReadStreamMock(); - TopicRpc rpc = Mockito.mock(TopicRpc.class); - Mockito.when(rpc.getScheduler()).thenReturn(Mockito.mock(ScheduledExecutorService.class)); - Mockito.when(rpc.readSession(Mockito.any(String.class))).thenReturn(stream); - - TopicClient client = TopicClientImpl.newClient(rpc).build(); - SyncReader reader = client.createSyncReader(ReaderSettings.newBuilder() - .addTopic(TopicReadSettings.newBuilder().setPath("/topic").build()) - .setConsumerName("consumer") - .withMeter(meter, "reader") - .build()); - - try { - reader.init(); - stream.assertLastMessage().isInitRequest("consumer", "/topic").hasReaderName("reader"); - stream.responseInit("read-session"); - stream.responseStartPartition("/topic", 42, 0); - byte[] msg1 = new byte[] { 1 }; - byte[] msg2 = new byte[] { 2 }; - stream.responseData(25).partition(1, 0) - .batch(Codec.RAW, msg1, msg2) - .and().send(); - Assert.assertEquals(2, meter.value(receivedMessages)); - Assert.assertEquals(25, meter.value(receivedBytes)); - - stream.responseData(30).partition(1, 2) - .batch(Codec.RAW, msg1, msg2, msg1) - .and().send(); - Assert.assertEquals(5, meter.value(receivedMessages)); - Assert.assertEquals(55, meter.value(receivedBytes)); - - Assert.assertEquals(0, meter.value(delivered)); - meter.assertAttribute(receivedMessages, "topic", "/topic"); - meter.assertAttribute(receivedMessages, "reader.name", "reader"); - - reader.receive(); - reader.receive(); - Assert.assertEquals(2, meter.value(delivered)); - } finally { - reader.shutdown(); - client.close(); + public void readerReconnectsThenStopsOnTerminalError() throws InterruptedException { + TopicMeterMock meter = new TopicMeterMock(); + ReadStreamMock first = new ReadStreamMock(); + ReadStreamMock retry = new ReadStreamMock(); + try (TopicClient client = TopicClientImpl.newClient(mockRpc(first, retry)).build()) { + SyncReader reader = client.createSyncReader(ReaderSettings.newBuilder() + .addTopic("/topic").setConsumerName("consumer").setMaxMemoryUsageBytes(100) + .setRetryConfig(status -> status.getCode() == StatusCode.TRANSPORT_UNAVAILABLE + ? (attempt, elapsed) -> 0 : null) + .withMeter(meter, "reader").build()); + try { + reader.init(); + first.responseInit("first"); + first.responseStartPartition("/topic", 42, 0); + first.responseData(25).partition(1, 0).batch(Codec.RAW, new byte[]{1}).and().send(); + reader.receive(); + first.closeStream(Status.of(StatusCode.TRANSPORT_UNAVAILABLE)); + Assert.assertEquals(1, meter.value(ERRORS)); + meter.assertAttribute(ERRORS, "retry_decision", "retry"); + meter.assertAttribute(ERRORS, "status_code", "TRANSPORT_UNAVAILABLE"); + meter.assertAttribute(ERRORS, "error.type", "transport_error"); + meter.assertNoGauges(); + Assert.assertEquals(1, meter.value(RECEIVED)); + Assert.assertEquals(1, meter.value(DELIVERED)); + Assert.assertEquals(25, meter.value(BYTES)); + + retry.responseInit("retry"); + retry.responseStartPartition("/topic", 42, 1); + Assert.assertEquals(1, meter.collect(PARTITIONS)); + Assert.assertEquals(100, meter.collect(CREDIT)); + retry.responseData(30).partition(1, 1).batch(Codec.RAW, new byte[]{2}).and().send(); + reader.receive(); + Assert.assertEquals(2, meter.value(RECEIVED)); + Assert.assertEquals(2, meter.value(DELIVERED)); + Assert.assertEquals(55, meter.value(BYTES)); + Assert.assertEquals(100, meter.collect(CREDIT)); + + retry.closeStream(Status.of(StatusCode.UNAUTHORIZED)); + Assert.assertEquals(2, meter.value(ERRORS)); + meter.assertAttribute(ERRORS, "retry_decision", "stop"); + meter.assertAttribute(ERRORS, "status_code", "UNAUTHORIZED"); + meter.assertAttribute(ERRORS, "error.type", "ydb_error"); + meter.assertAttribute(ERRORS, "consumer", "consumer"); + meter.assertAttribute(ERRORS, "reader.name", "reader"); + meter.assertNoGauges(); + } finally { + reader.shutdown(); + } } - Assert.assertEquals(0, meter.value("ydb.topic.reader.session.errors")); + Assert.assertEquals(2, meter.value(ERRORS)); + meter.assertNoGauges(); } - @Test - public void gaugesObservePartitionSessionsAndProtocolCredit() throws InterruptedException { - String partitions = "ydb.topic.reader.partition_session.count"; - String credit = "ydb.topic.reader.credit_balance_bytes"; - RecordingMeter meter = new RecordingMeter(); - ReadStreamMock stream = new ReadStreamMock(); + private static TopicRpc mockRpc(ReadStreamMock first, ReadStreamMock... retries) { TopicRpc rpc = Mockito.mock(TopicRpc.class); Mockito.when(rpc.getScheduler()).thenReturn(Mockito.mock(ScheduledExecutorService.class)); - Mockito.when(rpc.readSession(Mockito.anyString())).thenReturn(stream); - TopicClient client = TopicClientImpl.newClient(rpc).build(); - SyncReader reader = client.createSyncReader(ReaderSettings.newBuilder() - .addTopic("/topic").setConsumerName("consumer") - .setMaxMemoryUsageBytes(100).withMeter(meter, "reader").build()); - try { - Assert.assertTrue(meter.gauges.isEmpty()); - reader.init(); - Assert.assertTrue(meter.gauges.isEmpty()); - stream.responseInit("session"); - Assert.assertEquals(0, meter.collect(partitions)); - Assert.assertEquals(100, meter.collect(credit)); - stream.responseStartPartition("/topic", 42, 0); - Assert.assertEquals(1, meter.collect(partitions)); - Assert.assertEquals(100, meter.collect(credit)); - stream.responseData(20).partition(1, 0).batch(Codec.RAW, new byte[]{1}).and().send(); - Assert.assertEquals(80, meter.collect(credit)); - Assert.assertNotNull(reader.receive(1, TimeUnit.SECONDS)); - Assert.assertEquals(100, meter.collect(credit)); - stream.closeStream(Status.of(StatusCode.OVERLOADED)); - Assert.assertTrue(meter.gauges.isEmpty()); - } finally { - reader.shutdown(); - client.close(); - } - Assert.assertTrue(meter.gauges.isEmpty()); - } - - private static class RecordingMeter implements Meter { - private final Map counters = new ConcurrentHashMap<>(); - private final Map attributes = new ConcurrentHashMap<>(); - private final Map>> gauges = new ConcurrentHashMap<>(); - - @Override - public MetricRegistration registerLongGauge( - String name, String unit, String description, Consumer callback) { - List> callbacks = gauges.computeIfAbsent(name, key -> new ArrayList<>()); - callbacks.add(callback); - return () -> { - callbacks.remove(callback); - if (callbacks.isEmpty()) { - gauges.remove(name, callbacks); - } - }; - } - - long collect(String name) { - Long[] value = new Long[1]; - Assert.assertEquals(1, gauges.get(name).size()); - gauges.get(name).get(0).accept((observed, attrs) -> value[0] = observed); - return value[0]; - } - - @Override - public LongCounter createCounter(String name, String unit, String description) { - AtomicLong counter = counters.computeIfAbsent(name, key -> new AtomicLong()); - return (value, attrs) -> { - counter.addAndGet(value); - attributes.put(name, attrs); - }; - } - - long value(String name) { - AtomicLong counter = counters.get(name); - return counter == null ? 0 : counter.get(); - } - - void assertAttribute(String name, String key, String value) { - Assert.assertTrue(Arrays.stream(attributes.get(name)) - .anyMatch(attr -> attr.getKey().equals(key) && attr.getValue().equals(value))); - } + Mockito.when(rpc.readSession(Mockito.anyString())).thenReturn(first, retries); + return rpc; } } diff --git a/topic/src/test/java/tech/ydb/topic/utils/TopicMeterMock.java b/topic/src/test/java/tech/ydb/topic/utils/TopicMeterMock.java new file mode 100644 index 000000000..52a62263d --- /dev/null +++ b/topic/src/test/java/tech/ydb/topic/utils/TopicMeterMock.java @@ -0,0 +1,67 @@ +package tech.ydb.topic.utils; + +import java.util.Arrays; +import java.util.Map; +import java.util.concurrent.ConcurrentHashMap; +import java.util.concurrent.atomic.AtomicLong; +import java.util.function.Consumer; + +import org.junit.Assert; + +import tech.ydb.core.metrics.Attr; +import tech.ydb.core.metrics.LongCounter; +import tech.ydb.core.metrics.LongMeasurement; +import tech.ydb.core.metrics.Meter; +import tech.ydb.core.metrics.MetricRegistration; + +public class TopicMeterMock implements Meter { + private final Map counters = new ConcurrentHashMap<>(); + private final Map attributes = new ConcurrentHashMap<>(); + private final Map> gauges = new ConcurrentHashMap<>(); + + @Override + public LongCounter createCounter(String name, String unit, String description) { + AtomicLong counter = new AtomicLong(); + Assert.assertNull("Counter registered twice: " + name, counters.putIfAbsent(name, counter)); + return (value, attrs) -> { + attributes.put(name, attrs); + counter.addAndGet(value); + }; + } + + @Override + public MetricRegistration registerLongGauge( + String name, String unit, String description, Consumer callback) { + Assert.assertNull("Gauge registered twice: " + name, gauges.putIfAbsent(name, callback)); + return () -> gauges.remove(name, callback); + } + + public long value(String name) { + AtomicLong counter = counters.get(name); + Assert.assertNotNull("Counter not registered: " + name, counter); + return counter.get(); + } + + public long collect(String name) { + Consumer callback = gauges.get(name); + Assert.assertNotNull("Gauge not registered: " + name, callback); + Long[] value = new Long[1]; + callback.accept((observed, attrs) -> { + attributes.put(name, attrs); + value[0] = observed; + }); + Assert.assertNotNull("Gauge did not record: " + name, value[0]); + return value[0]; + } + + public void assertAttribute(String name, String key, String value) { + Attr[] attrs = attributes.get(name); + Assert.assertNotNull("Metric has no attributes: " + name, attrs); + Assert.assertTrue(name + ": missing " + key + "=" + value, + Arrays.stream(attrs).anyMatch(attr -> attr.getKey().equals(key) && attr.getValue().equals(value))); + } + + public void assertNoGauges() { + Assert.assertTrue("Gauges still registered: " + gauges.keySet(), gauges.isEmpty()); + } +} diff --git a/topic/src/test/java/tech/ydb/topic/write/impl/WriterMetricsTest.java b/topic/src/test/java/tech/ydb/topic/write/impl/WriterMetricsTest.java index 63589ecdf..70329498c 100644 --- a/topic/src/test/java/tech/ydb/topic/write/impl/WriterMetricsTest.java +++ b/topic/src/test/java/tech/ydb/topic/write/impl/WriterMetricsTest.java @@ -1,10 +1,7 @@ package tech.ydb.topic.write.impl; -import java.util.HashMap; -import java.util.Map; +import java.util.concurrent.CompletableFuture; import java.util.concurrent.ScheduledExecutorService; -import java.util.function.Consumer; -import java.util.function.LongSupplier; import org.junit.Assert; import org.junit.Test; @@ -12,59 +9,31 @@ import tech.ydb.core.Status; import tech.ydb.core.StatusCode; -import tech.ydb.core.metrics.LongCounter; -import tech.ydb.core.metrics.LongMeasurement; -import tech.ydb.core.metrics.Meter; -import tech.ydb.core.metrics.MetricRegistration; import tech.ydb.topic.TopicClient; import tech.ydb.topic.TopicRpc; import tech.ydb.topic.description.Codec; import tech.ydb.topic.impl.TopicClientImpl; import tech.ydb.topic.settings.WriterSettings; +import tech.ydb.topic.utils.TopicMeterMock; import tech.ydb.topic.write.AsyncWriter; import tech.ydb.topic.write.Message; import tech.ydb.topic.write.QueueOverflowException; +import tech.ydb.topic.write.WriteAck; public class WriterMetricsTest { + private static final String SENDING = "ydb.topic.writer.sending.messages"; + private static final String BYTES = "ydb.topic.writer.sending.bytes"; + private static final String WRITTEN = "ydb.topic.writer.written.messages"; + private static final String ERRORS = "ydb.topic.writer.session.errors"; + private static final String USED = "ydb.topic.writer.buffer.used.bytes"; + private static final String LIMIT = "ydb.topic.writer.buffer.limit.bytes"; @Test - public void sessionErrorsIncrementOnStreamFailure() { - Map counters = new HashMap<>(); - Meter meter = new Meter() { - @Override - public LongCounter createCounter(String name, String unit, String description) { - return (value, attrs) -> counters.merge(name, value, Long::sum); - } - }; + public void writerSendsMessagesAndShutsDown() throws QueueOverflowException { + TopicMeterMock meter = new TopicMeterMock(); WriteStreamMock stream = new WriteStreamMock(); WriterSettings settings = WriterSettings.newBuilder() - .setTopicPath("/test/topic").setCodec(Codec.RAW).setRetryConfig(status -> null) - .withMeter(meter, "writer").build(); - try (TopicClient client = TopicClientImpl.newClient(mockRpc(stream)).build()) { - AsyncWriter writer = client.createAsyncWriter(settings); - try { - writer.init(); - stream.close(Status.of(StatusCode.UNAUTHORIZED)); - Assert.assertEquals(Long.valueOf(1), counters.get("ydb.topic.writer.session.errors")); - } finally { - writer.shutdown(); - } - } - } - - @Test - public void writerCountersCountAcceptedMessagesAndAcknowledgements() throws QueueOverflowException { - Map counters = new HashMap<>(); - Meter meter = new Meter() { - @Override - public LongCounter createCounter(String name, String unit, String description) { - Assert.assertNull(counters.put(name, 0L)); - return (value, attrs) -> counters.put(name, counters.get(name) + value); - } - }; - WriteStreamMock stream = new WriteStreamMock(); - WriterSettings settings = WriterSettings.newBuilder() - .setTopicPath("/test/topic").setCodec(Codec.RAW) + .setTopicPath("/test/topic").setCodec(Codec.RAW).setMaxSendBufferMemorySize(100) .withMeter(meter, "writer").build(); try (TopicClient client = TopicClientImpl.newClient(mockRpc(stream)) .setCompressionExecutor(Runnable::run).build()) { @@ -72,67 +41,106 @@ public LongCounter createCounter(String name, String unit, String description) { try { writer.init(); stream.sendInitResponse(0); - writer.send(Message.of(new byte[]{1, 2})); - writer.send(Message.of(new byte[]{3, 4, 5})); + Assert.assertEquals(0, meter.value(SENDING)); + Assert.assertEquals(0, meter.value(WRITTEN)); + Assert.assertEquals(0, meter.value(ERRORS)); + Assert.assertEquals(0, meter.collect(USED)); + Assert.assertEquals(100, meter.collect(LIMIT)); + + CompletableFuture first = writer.send(Message.of(new byte[]{1, 2})); + CompletableFuture second = writer.send(Message.of(new byte[]{3, 4, 5})); + Assert.assertFalse(first.isDone()); + Assert.assertFalse(second.isDone()); + Assert.assertEquals(2, meter.value(SENDING)); + Assert.assertEquals(5, meter.value(BYTES)); + Assert.assertEquals(0, meter.value(WRITTEN)); + Assert.assertEquals(5, meter.collect(USED)); + Assert.assertEquals(100, meter.collect(LIMIT)); + stream.sendAckResponse(2, 10); - Assert.assertEquals(Long.valueOf(2), counters.get("ydb.topic.writer.sending.messages")); - Assert.assertEquals(Long.valueOf(5), counters.get("ydb.topic.writer.sending.bytes")); - Assert.assertEquals(Long.valueOf(2), counters.get("ydb.topic.writer.written.messages")); + Assert.assertTrue(first.isDone()); + Assert.assertTrue(second.isDone()); + Assert.assertEquals(2, meter.value(SENDING)); + Assert.assertEquals(5, meter.value(BYTES)); + Assert.assertEquals(2, meter.value(WRITTEN)); + Assert.assertEquals(0, meter.value(ERRORS)); + Assert.assertEquals(0, meter.collect(USED)); + meter.assertAttribute(SENDING, "topic", "/test/topic"); + meter.assertAttribute(BYTES, "writer.name", "writer"); + meter.assertAttribute(WRITTEN, "writer.name", "writer"); + meter.assertAttribute(USED, "topic", "/test/topic"); + meter.assertAttribute(LIMIT, "writer.name", "writer"); } finally { writer.shutdown(); stream.close(Status.SUCCESS); } } + Assert.assertEquals(0, meter.value(ERRORS)); + meter.assertNoGauges(); } @Test - public void writerBufferGaugesTrackBudgetAndLimit() throws QueueOverflowException { - Map gauges = new HashMap<>(); - Meter meter = new Meter() { - @Override - public MetricRegistration registerLongGauge(String name, String unit, String description, - Consumer callback) { - gauges.put(name, () -> { - long[] value = new long[1]; - callback.accept((observed, attrs) -> value[0] = observed); - return value[0]; - }); - return () -> gauges.remove(name); - } - }; - String used = "ydb.topic.writer.buffer.used.bytes"; - String limit = "ydb.topic.writer.buffer.limit.bytes"; - WriteStreamMock stream = new WriteStreamMock(); + public void writerReconnectsThenStopsOnTerminalError() throws QueueOverflowException { + TopicMeterMock meter = new TopicMeterMock(); + WriteStreamMock first = new WriteStreamMock(); + WriteStreamMock retry = new WriteStreamMock(); WriterSettings settings = WriterSettings.newBuilder() .setTopicPath("/test/topic").setCodec(Codec.RAW).setMaxSendBufferMemorySize(100) + .setRetryConfig(status -> status.getCode() == StatusCode.TRANSPORT_UNAVAILABLE + ? (attempt, elapsed) -> 0 : null) .withMeter(meter, "writer").build(); - try (TopicClient client = TopicClientImpl.newClient(mockRpc(stream)) + try (TopicClient client = TopicClientImpl.newClient(mockRpc(first, retry)) .setCompressionExecutor(Runnable::run).build()) { AsyncWriter writer = client.createAsyncWriter(settings); try { writer.init(); - stream.sendInitResponse(0); - Assert.assertEquals(0, gauges.get(used).getAsLong()); - Assert.assertEquals(100, gauges.get(limit).getAsLong()); - writer.send(Message.of(new byte[]{1, 2})); - writer.send(Message.of(new byte[]{3, 4, 5})); - Assert.assertEquals(5, gauges.get(used).getAsLong()); - Assert.assertEquals(100, gauges.get(limit).getAsLong()); - stream.sendAckResponse(2, 10); - Assert.assertEquals(0, gauges.get(used).getAsLong()); + first.sendInitResponse(0); + CompletableFuture written = writer.send(Message.of(new byte[]{1, 2})); + CompletableFuture pending = writer.send(Message.of(new byte[]{3, 4, 5})); + first.close(Status.of(StatusCode.TRANSPORT_UNAVAILABLE)); + Assert.assertEquals(1, meter.value(ERRORS)); + meter.assertAttribute(ERRORS, "retry_decision", "retry"); + meter.assertAttribute(ERRORS, "status_code", "TRANSPORT_UNAVAILABLE"); + meter.assertAttribute(ERRORS, "error.type", "transport_error"); + Assert.assertEquals(2, meter.value(SENDING)); + Assert.assertEquals(5, meter.value(BYTES)); + Assert.assertEquals(0, meter.value(WRITTEN)); + Assert.assertEquals(5, meter.collect(USED)); + Assert.assertEquals(100, meter.collect(LIMIT)); + + retry.sendInitResponse(0); + retry.sendAckResponse(1, 10); + Assert.assertTrue(written.isDone()); + Assert.assertFalse(pending.isDone()); + Assert.assertEquals(2, meter.value(SENDING)); + Assert.assertEquals(5, meter.value(BYTES)); + Assert.assertEquals(1, meter.value(WRITTEN)); + Assert.assertEquals(3, meter.collect(USED)); + + retry.close(Status.of(StatusCode.UNAUTHORIZED)); + Assert.assertTrue(pending.isCompletedExceptionally()); + Assert.assertEquals(2, meter.value(ERRORS)); + Assert.assertEquals(2, meter.value(SENDING)); + Assert.assertEquals(5, meter.value(BYTES)); + Assert.assertEquals(1, meter.value(WRITTEN)); + meter.assertAttribute(ERRORS, "retry_decision", "stop"); + meter.assertAttribute(ERRORS, "status_code", "UNAUTHORIZED"); + meter.assertAttribute(ERRORS, "error.type", "ydb_error"); + meter.assertAttribute(ERRORS, "topic", "/test/topic"); + meter.assertAttribute(ERRORS, "writer.name", "writer"); + meter.assertNoGauges(); } finally { writer.shutdown(); - stream.close(Status.SUCCESS); } } - Assert.assertTrue(gauges.isEmpty()); + Assert.assertEquals(2, meter.value(ERRORS)); + meter.assertNoGauges(); } - private static TopicRpc mockRpc(WriteStreamMock stream) { + private static TopicRpc mockRpc(WriteStreamMock first, WriteStreamMock... retries) { TopicRpc rpc = Mockito.mock(TopicRpc.class); Mockito.when(rpc.getScheduler()).thenReturn(Mockito.mock(ScheduledExecutorService.class)); - Mockito.when(rpc.writeSession(Mockito.any(String.class))).thenReturn(stream); + Mockito.when(rpc.writeSession(Mockito.anyString())).thenReturn(first, retries); return rpc; } - } diff --git a/topic/src/test/java/tech/ydb/topic/write/impl/WriterQueueTest.java b/topic/src/test/java/tech/ydb/topic/write/impl/WriterQueueTest.java index d4bacf56e..8a067ba71 100644 --- a/topic/src/test/java/tech/ydb/topic/write/impl/WriterQueueTest.java +++ b/topic/src/test/java/tech/ydb/topic/write/impl/WriterQueueTest.java @@ -19,7 +19,6 @@ import tech.ydb.core.Status; import tech.ydb.core.StatusCode; -import tech.ydb.core.metrics.Meter; import tech.ydb.topic.description.Codec; import tech.ydb.topic.description.CodecRegistry; import tech.ydb.topic.settings.WriterSettings; @@ -59,14 +58,10 @@ private static WriterSettings gzipSettings() { } private static WriterQueue rawQueue(AtomicInteger notifyCount) { - return new WriterQueue("test", rawSettings(), metrics(), new CodecRegistry(), + return new WriterQueue("test", rawSettings(), new CodecRegistry(), Runnable::run, () -> notifyCount.incrementAndGet()); } - private static WriterMetrics metrics() { - return new WriterMetrics(Meter.NOOP, "/test", null); - } - private static void assertOverflow(String msg, ThrowingRunnable runnable) { QueueOverflowException ex = Assert.assertThrows("Must be thrown QueueOverflowException", QueueOverflowException.class, runnable); @@ -93,7 +88,7 @@ public void testInvalidCodecThrows() { .setCodec(9999) .build(); IllegalArgumentException ex = Assert.assertThrows(IllegalArgumentException.class, - () -> new WriterQueue("test", settings, metrics(), new CodecRegistry(), Runnable::run, () -> { })); + () -> new WriterQueue("test", settings, new CodecRegistry(), Runnable::run, () -> { })); Assert.assertEquals("Unsupported codec: 9999", ex.getMessage()); } @@ -107,7 +102,7 @@ public void testFlushWithNoMessages() { @Test public void testRawCompressor() throws Exception { - WriterQueue q = new WriterQueue("test", rawSettings(), metrics(), new CodecRegistry(), null, () -> {}); + WriterQueue q = new WriterQueue("test", rawSettings(), new CodecRegistry(), null, () -> {}); CompletableFuture f1 = q.enqueue(SMALL_MSG, null); CompletableFuture f2 = q.tryEnqueue(SMALL_MSG, null); @@ -138,7 +133,7 @@ public void testCompressedExpansionIsAccounted() throws Exception { .setCodec(codec.getId()) .setMaxSendBufferMemorySize(10) .build(); - WriterQueue writerQueue = new WriterQueue("test", settings, metrics(), new CodecRegistry(singletonList(codec)), + WriterQueue writerQueue = new WriterQueue("test", settings, new CodecRegistry(singletonList(codec)), Runnable::run, () -> { }); @@ -158,8 +153,7 @@ public void testLzopFramingIsAccounted() throws Exception { .setCodec(Codec.LZOP) .setMaxSendBufferMemorySize(109) .build(); - WriterQueue writerQueue = new WriterQueue("test", settings, metrics(), new CodecRegistry(), encodingTasks::add, - () -> { + WriterQueue writerQueue = new WriterQueue("test", settings, new CodecRegistry(), encodingTasks::add, () -> { }); writerQueue.tryEnqueue(SMALL_MSG, null); @@ -189,7 +183,7 @@ public void testEncodedMessageLargerThanBufferIsProcessed() throws Exception { .setCodec(codec.getId()) .setMaxSendBufferMemorySize(10) .build(); - WriterQueue writerQueue = new WriterQueue("test", settings, metrics(), new CodecRegistry(singletonList(codec)), + WriterQueue writerQueue = new WriterQueue("test", settings, new CodecRegistry(singletonList(codec)), encodingTasks::add, () -> { }); @@ -220,7 +214,7 @@ public void testExpansionUsesAvailableCapacity() throws Exception { .setCodec(codec.getId()) .setMaxSendBufferMemorySize(12) .build(); - WriterQueue writerQueue = new WriterQueue("test", settings, metrics(), new CodecRegistry(singletonList(codec)), + WriterQueue writerQueue = new WriterQueue("test", settings, new CodecRegistry(singletonList(codec)), encodingTasks::add, () -> { }); @@ -244,7 +238,7 @@ public void testExpansionUsesAvailableCapacity() throws Exception { @Test @HideLoggers({ WriterImpl.class }) public void testGzipNullCompressor() throws Exception { - WriterQueue q = new WriterQueue("test", gzipSettings(), metrics(), new CodecRegistry(), null, () -> {}); + WriterQueue q = new WriterQueue("test", gzipSettings(), new CodecRegistry(), null, () -> {}); CompletableFuture f1 = q.enqueue(SMALL_MSG, null); CompletableFuture f2 = q.tryEnqueue(SMALL_MSG, null); @@ -289,7 +283,7 @@ public OutputStream encode(OutputStream out) throws IOException { .setCodec(failingCodec.getId()) .build(); - WriterQueue q = new WriterQueue("test", settings, metrics(), registry, Runnable::run, notify::incrementAndGet); + WriterQueue q = new WriterQueue("test", settings, registry, Runnable::run, notify::incrementAndGet); CompletableFuture f1 = q.enqueue(SMALL_MSG, null); CompletableFuture f2 = q.tryEnqueue(SMALL_MSG, null); @@ -341,7 +335,7 @@ public void write(int b) throws IOException { .build(); AtomicInteger notify = new AtomicInteger(); CodecRegistry registry = new CodecRegistry(Arrays.asList(selective)); - WriterQueue q = new WriterQueue("test", settings, metrics(), registry, Runnable::run, notify::incrementAndGet); + WriterQueue q = new WriterQueue("test", settings, registry, Runnable::run, notify::incrementAndGet); CompletableFuture good1 = q.enqueue(Message.of(new byte[] { 0x01 }), null); CompletableFuture bad = q.enqueue(Message.of(new byte[] { 0x7F }), null); @@ -412,7 +406,7 @@ public void testSmallBufferWriting() throws QueueOverflowException { .setMaxSendBufferMemorySize(12) .build(); - WriterQueue q = new WriterQueue("test", settings, metrics(), new CodecRegistry(), null, notify::incrementAndGet); + WriterQueue q = new WriterQueue("test", settings, new CodecRegistry(), null, notify::incrementAndGet); q.tryEnqueue(smallMsg(10), null); // success q.tryEnqueue(smallMsg(20), null); // success From 9726c47e0be5a9ab5ee18c8b1543098d3c4e5bd3 Mon Sep 17 00:00:00 2001 From: KirillKurdyukov Date: Thu, 15 Oct 2026 22:32:46 +0300 Subject: [PATCH 4/9] Distinguish Reader session error origins --- .../ydb/topic/impl/TopicRetryableStream.java | 33 ++- .../java/tech/ydb/topic/impl/TopicStream.java | 4 + .../tech/ydb/topic/impl/TopicStreamBase.java | 33 ++- .../ydb/topic/read/impl/AsyncReaderImpl.java | 3 +- .../tech/ydb/topic/read/impl/ReadSession.java | 4 +- .../tech/ydb/topic/read/impl/ReaderImpl.java | 20 +- .../ydb/topic/read/impl/ReaderMetrics.java | 7 +- .../topic/read/impl/ReaderMetricsTest.java | 245 ++++++++++-------- .../tech/ydb/topic/utils/TopicMeterMock.java | 67 ----- .../topic/write/impl/WriterMetricsTest.java | 144 ++++------ 10 files changed, 269 insertions(+), 291 deletions(-) delete mode 100644 topic/src/test/java/tech/ydb/topic/utils/TopicMeterMock.java diff --git a/topic/src/main/java/tech/ydb/topic/impl/TopicRetryableStream.java b/topic/src/main/java/tech/ydb/topic/impl/TopicRetryableStream.java index c8ac9c174..f78317ecf 100644 --- a/topic/src/main/java/tech/ydb/topic/impl/TopicRetryableStream.java +++ b/topic/src/main/java/tech/ydb/topic/impl/TopicRetryableStream.java @@ -60,6 +60,14 @@ public TopicRetryableStream(Logger logger, String debugId, RetryConfig config, S */ protected abstract void onClose(@Nullable S stream, Status status); + protected void onRetry(@Nullable S stream, Status status, String failureReason) { + onRetry(stream, status); + } + + protected void onClose(@Nullable S stream, Status status, String failureReason) { + onClose(stream, status); + } + public void start() { if (isClosed) { logger.warn("[{}] double start of closed stream, ignored", debugId); @@ -91,10 +99,14 @@ public boolean isClosed() { } public void fail(Status status) { + fail(status, null); + } + + public void fail(Status status, String failureReason) { State local = state.getAndSet(null); if (local != null) { logger.warn("[{}] failed by application-side error {}", debugId, status); - onStreamStop(local.closeStream(), status, retryConfig.getStatusRetryPolicy(status)); + onStreamStop(local.closeStream(), status, retryConfig.getStatusRetryPolicy(status), failureReason); } } @@ -120,15 +132,19 @@ public boolean close() { } private void onStreamStop(S closed, Status status, RetryPolicy policy) { + onStreamStop(closed, status, policy, null); + } + + private void onStreamStop(S closed, Status status, RetryPolicy policy, String failureReason) { if (isClosed) { // stream was already closed (usually with success) - onClose(closed, status); + onClose(closed, status, failureReason); return; } if (policy == null) { logger.warn("[{}] stopped by non-retryable status {}", debugId, status); isClosed = true; - onClose(closed, status); + onClose(closed, status, failureReason); return; } @@ -137,27 +153,27 @@ private void onStreamStop(S closed, Status status, RetryPolicy policy) { if (nextRetryMs < 0) { logger.warn("[{}] stopped after retry policy evaluation for status {}", debugId, status); isClosed = true; - onClose(closed, status); + onClose(closed, status, failureReason); return; } if (nextRetryMs == 0) { // retry immediately logger.warn("[{}] retry #{}. Retry immediately...", debugId, retryState.retryNumber()); - onRetry(closed, status); + onRetry(closed, status, failureReason); start(); return; } // retry scheduling logger.warn("[{}] retry #{}. Scheduling reconnect in {}ms...", debugId, retryState.retryNumber(), nextRetryMs); - onRetry(closed, status); + onRetry(closed, status, failureReason); try { scheduler.schedule(this::start, nextRetryMs, TimeUnit.MILLISECONDS); } catch (Exception ex) { logger.error("[{}] cannot schedule reconnect, stopping", debugId, ex); isClosed = true; - onClose(closed, status); + onClose(closed, status, failureReason); } } @@ -211,7 +227,8 @@ private void startStream(S local) { } if (status != null) { - onStreamStop(local, status, retryConfig.getStatusRetryPolicy(status)); + onStreamStop(local, status, retryConfig.getStatusRetryPolicy(status), + local.getFailureReason(status)); } else { Status wrapped = Status.of(StatusCode.CLIENT_INTERNAL_ERROR, th); onStreamStop(local, wrapped, retryConfig.getThrowableRetryPolicy(th)); diff --git a/topic/src/main/java/tech/ydb/topic/impl/TopicStream.java b/topic/src/main/java/tech/ydb/topic/impl/TopicStream.java index 75236073b..856065601 100644 --- a/topic/src/main/java/tech/ydb/topic/impl/TopicStream.java +++ b/topic/src/main/java/tech/ydb/topic/impl/TopicStream.java @@ -10,6 +10,10 @@ public interface TopicStream { CompletableFuture start(Consumer messageHandler); + default String getFailureReason(Status status) { + return null; + } + void send(W request); void close(); } diff --git a/topic/src/main/java/tech/ydb/topic/impl/TopicStreamBase.java b/topic/src/main/java/tech/ydb/topic/impl/TopicStreamBase.java index 522143003..4dc4cdd18 100644 --- a/topic/src/main/java/tech/ydb/topic/impl/TopicStreamBase.java +++ b/topic/src/main/java/tech/ydb/topic/impl/TopicStreamBase.java @@ -16,7 +16,8 @@ public abstract class TopicStreamBase impl private final String debugId; private final GrpcReadWriteStream stream; private final W initRequest; - private final CompletableFuture streamStatus = new CompletableFuture<>(); + private final CompletableFuture completion = new CompletableFuture<>(); + private final CompletableFuture streamStatus = completion.thenApply(value -> value.status); private volatile String token; private volatile boolean isStopped = false; @@ -40,7 +41,7 @@ public CompletableFuture start(Consumer messageHandler) { messageHandler.accept(msg); } else { logger.warn("[{}] stopped by getting status {}", debugId, messageStatus); - if (streamStatus.complete(messageStatus)) { + if (completion.complete(new Completion(messageStatus, null))) { stream.close(); } } @@ -48,10 +49,10 @@ public CompletableFuture start(Consumer messageHandler) { isStopped = true; Status status = st != null ? st : Status.of(StatusCode.CLIENT_INTERNAL_ERROR, th); logger.debug("[{}] finished with status {}", debugId, status); - streamStatus.complete(status); + completion.complete(new Completion(status, null)); }); - if (!streamStatus.isDone()) { + if (!completion.isDone()) { stream.sendNext(initRequest); } @@ -61,20 +62,30 @@ public CompletableFuture start(Consumer messageHandler) { @Override public void close() { logger.debug("[{}] closed by app", debugId); - if (!streamStatus.isDone()) { + if (!completion.isDone()) { isStopped = true; stream.close(); } } protected void fail(Status status) { + fail(status, null); + } + + protected void fail(Status status, String reason) { logger.warn("[{}] stopped by fail {}", debugId, status); - if (streamStatus.complete(status)) { + if (completion.complete(new Completion(status, reason))) { isStopped = true; stream.close(); } } + @Override + public String getFailureReason(Status status) { + Completion value = completion.getNow(null); + return value != null && value.status == status ? value.reason : null; + } + @Override public void send(W req) { if (isStopped) { @@ -92,4 +103,14 @@ public void send(W req) { stream.sendNext(req); } + + private static final class Completion { + private final Status status; + private final String reason; + + Completion(Status status, String reason) { + this.status = status; + this.reason = reason; + } + } } diff --git a/topic/src/main/java/tech/ydb/topic/read/impl/AsyncReaderImpl.java b/topic/src/main/java/tech/ydb/topic/read/impl/AsyncReaderImpl.java index d5497f2f3..966dbb315 100644 --- a/topic/src/main/java/tech/ydb/topic/read/impl/AsyncReaderImpl.java +++ b/topic/src/main/java/tech/ydb/topic/read/impl/AsyncReaderImpl.java @@ -118,7 +118,8 @@ private void close(Status status) { private void failSession(Throwable th, String callbackName) { String errorMessage = "Unhandled throwable in " + callbackName + " user callback: " + th.getMessage(); logger.error(errorMessage, th); - impl.fail(Status.of(StatusCode.CLIENT_INTERNAL_ERROR, th, Issue.of(errorMessage, Issue.Severity.ERROR))); + impl.fail(Status.of(StatusCode.CLIENT_INTERNAL_ERROR, th, Issue.of(errorMessage, Issue.Severity.ERROR)), + "user_callback_error"); } private class Impl extends ReaderImpl { diff --git a/topic/src/main/java/tech/ydb/topic/read/impl/ReadSession.java b/topic/src/main/java/tech/ydb/topic/read/impl/ReadSession.java index bd9c30800..437a942b3 100644 --- a/topic/src/main/java/tech/ydb/topic/read/impl/ReadSession.java +++ b/topic/src/main/java/tech/ydb/topic/read/impl/ReadSession.java @@ -183,7 +183,7 @@ public void onStartPartition(YdbTopic.StreamReadMessage.StartPartitionSessionReq logger.error("[{}] Received second StartPartitionSessionRequest for the already active {}", debugId, ps); Issue issue = Issue.of("Restarting read session due to receiving second StartPartitionSessionRequest with " + ps, Issue.Severity.FATAL); - fail(Status.of(StatusCode.CLIENT_INTERNAL_ERROR, issue)); + fail(Status.of(StatusCode.CLIENT_INTERNAL_ERROR, issue), "protocol_error"); return; } @@ -215,7 +215,7 @@ public void onStopPartition(YdbTopic.StreamReadMessage.StopPartitionSessionReque "but have no such partition session active", debugId, psid); Issue issue = Issue.of("Restarting read session due to receiving StopPartitionSessionRequest with " + "PartitionSessionId " + psid + " that SDK knows nothing about", Issue.Severity.FATAL); - fail(Status.of(StatusCode.CLIENT_INTERNAL_ERROR, issue)); + fail(Status.of(StatusCode.CLIENT_INTERNAL_ERROR, issue), "protocol_error"); } public void onRead(YdbTopic.StreamReadMessage.ReadResponse response) { diff --git a/topic/src/main/java/tech/ydb/topic/read/impl/ReaderImpl.java b/topic/src/main/java/tech/ydb/topic/read/impl/ReaderImpl.java index 9f41b933c..6ab2f03a9 100644 --- a/topic/src/main/java/tech/ydb/topic/read/impl/ReaderImpl.java +++ b/topic/src/main/java/tech/ydb/topic/read/impl/ReaderImpl.java @@ -67,7 +67,12 @@ protected CompletableFuture> createNewStream(String id) { @Override protected void onRetry(ReadSession stream, Status status) { - config.getMetrics().reportSessionError(status, true); + onRetry(stream, status, null); + } + + @Override + protected void onRetry(ReadSession stream, Status status, String failureReason) { + config.getMetrics().reportSessionError(status, true, failureReason); logger.warn("[{}] stopped by status {}", debugId, status); currentSessionId = null; if (errorHandler != null) { @@ -84,9 +89,14 @@ protected void onRetry(ReadSession stream, Status status) { @Override protected void onClose(ReadSession stream, Status status) { + onClose(stream, status, null); + } + + @Override + protected void onClose(ReadSession stream, Status status, String failureReason) { currentSessionId = null; if (!status.isSuccess()) { - config.getMetrics().reportSessionError(status, false); + config.getMetrics().reportSessionError(status, false, failureReason); logger.warn("[{}] closed by status {}", debugId, status); } else { logger.info("[{}] closed by status {}", debugId, status); @@ -178,7 +188,8 @@ public CompletableFuture updateOffsetsInTransaction(YdbTransaction trans " with partition offsets from read session " + debugId + " was not committed with status: " + status; if (sessionId == currentSessionId) { - fail(Status.of(StatusCode.CLIENT_INTERNAL_ERROR, Issue.of(msg, Issue.Severity.ERROR))); + fail(Status.of(StatusCode.CLIENT_INTERNAL_ERROR, Issue.of(msg, Issue.Severity.ERROR)), + "transaction_error"); } } if (error != null) { @@ -186,7 +197,8 @@ public CompletableFuture updateOffsetsInTransaction(YdbTransaction trans " with partition offsets from read session " + debugId + " was not committed with reason: " + error.getMessage(); if (sessionId == currentSessionId) { - fail(Status.of(StatusCode.CLIENT_INTERNAL_ERROR, Issue.of(msg, Issue.Severity.ERROR))); + fail(Status.of(StatusCode.CLIENT_INTERNAL_ERROR, Issue.of(msg, Issue.Severity.ERROR)), + "transaction_error"); } } }); diff --git a/topic/src/main/java/tech/ydb/topic/read/impl/ReaderMetrics.java b/topic/src/main/java/tech/ydb/topic/read/impl/ReaderMetrics.java index 0a8804304..c61a780fe 100644 --- a/topic/src/main/java/tech/ydb/topic/read/impl/ReaderMetrics.java +++ b/topic/src/main/java/tech/ydb/topic/read/impl/ReaderMetrics.java @@ -73,13 +73,14 @@ void reportReceivedBytes(long bytes) { } } - void reportSessionError(Status status, boolean retry) { + void reportSessionError(Status status, boolean retry, String failureReason) { StatusCode code = status.getCode(); Attr[] attributes = Arrays.copyOf(commonAttributes, commonAttributes.length + 3); attributes[commonAttributes.length] = Attr.of("retry_decision", retry ? "retry" : "stop"); attributes[commonAttributes.length + 1] = Attr.of("status_code", code.name()); - String errorType = code == StatusCode.SUCCESS || code == StatusCode.UNUSED_STATUS - ? "session_closed" : code.isTransportError() ? "transport_error" : "ydb_error"; + String errorType = failureReason != null ? failureReason + : code == StatusCode.SUCCESS || code == StatusCode.UNUSED_STATUS + ? "session_closed" : code.isTransportError() ? "transport_error" : "ydb_error"; attributes[commonAttributes.length + 2] = Attr.of("error.type", errorType); sessionErrors.add(1, attributes); } diff --git a/topic/src/test/java/tech/ydb/topic/read/impl/ReaderMetricsTest.java b/topic/src/test/java/tech/ydb/topic/read/impl/ReaderMetricsTest.java index 61d3a8ac0..c300e764d 100644 --- a/topic/src/test/java/tech/ydb/topic/read/impl/ReaderMetricsTest.java +++ b/topic/src/test/java/tech/ydb/topic/read/impl/ReaderMetricsTest.java @@ -1,6 +1,14 @@ package tech.ydb.topic.read.impl; +import java.util.ArrayList; +import java.util.Arrays; +import java.util.List; +import java.util.Map; +import java.util.concurrent.ConcurrentHashMap; import java.util.concurrent.ScheduledExecutorService; +import java.util.concurrent.TimeUnit; +import java.util.concurrent.atomic.AtomicLong; +import java.util.function.Consumer; import org.junit.Assert; import org.junit.Test; @@ -8,134 +16,149 @@ import tech.ydb.core.Status; import tech.ydb.core.StatusCode; +import tech.ydb.core.metrics.Attr; +import tech.ydb.core.metrics.LongCounter; +import tech.ydb.core.metrics.LongMeasurement; +import tech.ydb.core.metrics.Meter; +import tech.ydb.core.metrics.MetricRegistration; import tech.ydb.topic.TopicClient; import tech.ydb.topic.TopicRpc; import tech.ydb.topic.description.Codec; import tech.ydb.topic.impl.TopicClientImpl; import tech.ydb.topic.read.SyncReader; import tech.ydb.topic.settings.ReaderSettings; -import tech.ydb.topic.utils.TopicMeterMock; +import tech.ydb.topic.settings.TopicReadSettings; public class ReaderMetricsTest { - private static final String DELIVERED = "ydb.topic.reader.delivered.messages"; - private static final String RECEIVED = "ydb.topic.reader.received.messages"; - private static final String BYTES = "ydb.topic.reader.received.bytes"; - private static final String ERRORS = "ydb.topic.reader.session.errors"; - private static final String PARTITIONS = "ydb.topic.reader.partition_session.count"; - private static final String CREDIT = "ydb.topic.reader.credit_balance_bytes"; @Test - public void readerReceivesMessagesAndShutsDown() throws InterruptedException { - TopicMeterMock meter = new TopicMeterMock(); + public void readerCountersIncrementOnReceive() throws InterruptedException { + String delivered = "ydb.topic.reader.delivered.messages"; + String receivedMessages = "ydb.topic.reader.received.messages"; + String receivedBytes = "ydb.topic.reader.received.bytes"; + RecordingMeter meter = new RecordingMeter(); ReadStreamMock stream = new ReadStreamMock(); - try (TopicClient client = TopicClientImpl.newClient(mockRpc(stream)).build()) { - SyncReader reader = client.createSyncReader(ReaderSettings.newBuilder() - .addTopic("/topic").setConsumerName("consumer") - .setMaxMemoryUsageBytes(100).withMeter(meter, "reader").build()); - try { - meter.assertNoGauges(); - reader.init(); - stream.assertLastMessage().isInitRequest("consumer", "/topic").hasReaderName("reader"); - meter.assertNoGauges(); - stream.responseInit("session"); - Assert.assertEquals(0, meter.collect(PARTITIONS)); - Assert.assertEquals(100, meter.collect(CREDIT)); - stream.responseStartPartition("/topic", 42, 0); - Assert.assertEquals(1, meter.collect(PARTITIONS)); - - byte[] msg1 = new byte[]{1}; - byte[] msg2 = new byte[]{2}; - stream.responseData(25).partition(1, 0).batch(Codec.RAW, msg1, msg2).and().send(); - Assert.assertEquals(2, meter.value(RECEIVED)); - Assert.assertEquals(25, meter.value(BYTES)); - Assert.assertEquals(75, meter.collect(CREDIT)); - Assert.assertEquals(0, meter.value(DELIVERED)); - Assert.assertEquals(0, meter.value(ERRORS)); - - stream.responseData(30).partition(1, 2).batch(Codec.RAW, msg1, msg2, msg1).and().send(); - Assert.assertEquals(5, meter.value(RECEIVED)); - Assert.assertEquals(55, meter.value(BYTES)); - Assert.assertEquals(45, meter.collect(CREDIT)); - reader.receive(); - reader.receive(); - Assert.assertEquals(2, meter.value(DELIVERED)); - reader.receive(); - reader.receive(); - reader.receive(); - Assert.assertEquals(5, meter.value(DELIVERED)); - Assert.assertEquals(100, meter.collect(CREDIT)); - Assert.assertEquals(1, meter.collect(PARTITIONS)); - meter.assertAttribute(RECEIVED, "topic", "/topic"); - meter.assertAttribute(DELIVERED, "topic", "/topic"); - meter.assertAttribute(BYTES, "consumer", "consumer"); - meter.assertAttribute(PARTITIONS, "reader.name", "reader"); - meter.assertAttribute(CREDIT, "consumer", "consumer"); - } finally { - reader.shutdown(); - } + TopicRpc rpc = Mockito.mock(TopicRpc.class); + Mockito.when(rpc.getScheduler()).thenReturn(Mockito.mock(ScheduledExecutorService.class)); + Mockito.when(rpc.readSession(Mockito.any(String.class))).thenReturn(stream); + + TopicClient client = TopicClientImpl.newClient(rpc).build(); + SyncReader reader = client.createSyncReader(ReaderSettings.newBuilder() + .addTopic(TopicReadSettings.newBuilder().setPath("/topic").build()) + .setConsumerName("consumer") + .withMeter(meter, "reader") + .build()); + + try { + reader.init(); + stream.assertLastMessage().isInitRequest("consumer", "/topic").hasReaderName("reader"); + stream.responseInit("read-session"); + stream.responseStartPartition("/topic", 42, 0); + byte[] msg1 = new byte[] { 1 }; + byte[] msg2 = new byte[] { 2 }; + stream.responseData(25).partition(1, 0) + .batch(Codec.RAW, msg1, msg2) + .and().send(); + Assert.assertEquals(2, meter.value(receivedMessages)); + Assert.assertEquals(25, meter.value(receivedBytes)); + + stream.responseData(30).partition(1, 2) + .batch(Codec.RAW, msg1, msg2, msg1) + .and().send(); + Assert.assertEquals(5, meter.value(receivedMessages)); + Assert.assertEquals(55, meter.value(receivedBytes)); + + Assert.assertEquals(0, meter.value(delivered)); + meter.assertAttribute(receivedMessages, "topic", "/topic"); + meter.assertAttribute(receivedMessages, "reader.name", "reader"); + + reader.receive(); + reader.receive(); + Assert.assertEquals(2, meter.value(delivered)); + } finally { + reader.shutdown(); + client.close(); } - Assert.assertEquals(0, meter.value(ERRORS)); - meter.assertNoGauges(); } @Test - public void readerReconnectsThenStopsOnTerminalError() throws InterruptedException { - TopicMeterMock meter = new TopicMeterMock(); - ReadStreamMock first = new ReadStreamMock(); - ReadStreamMock retry = new ReadStreamMock(); - try (TopicClient client = TopicClientImpl.newClient(mockRpc(first, retry)).build()) { - SyncReader reader = client.createSyncReader(ReaderSettings.newBuilder() - .addTopic("/topic").setConsumerName("consumer").setMaxMemoryUsageBytes(100) - .setRetryConfig(status -> status.getCode() == StatusCode.TRANSPORT_UNAVAILABLE - ? (attempt, elapsed) -> 0 : null) - .withMeter(meter, "reader").build()); - try { - reader.init(); - first.responseInit("first"); - first.responseStartPartition("/topic", 42, 0); - first.responseData(25).partition(1, 0).batch(Codec.RAW, new byte[]{1}).and().send(); - reader.receive(); - first.closeStream(Status.of(StatusCode.TRANSPORT_UNAVAILABLE)); - Assert.assertEquals(1, meter.value(ERRORS)); - meter.assertAttribute(ERRORS, "retry_decision", "retry"); - meter.assertAttribute(ERRORS, "status_code", "TRANSPORT_UNAVAILABLE"); - meter.assertAttribute(ERRORS, "error.type", "transport_error"); - meter.assertNoGauges(); - Assert.assertEquals(1, meter.value(RECEIVED)); - Assert.assertEquals(1, meter.value(DELIVERED)); - Assert.assertEquals(25, meter.value(BYTES)); - - retry.responseInit("retry"); - retry.responseStartPartition("/topic", 42, 1); - Assert.assertEquals(1, meter.collect(PARTITIONS)); - Assert.assertEquals(100, meter.collect(CREDIT)); - retry.responseData(30).partition(1, 1).batch(Codec.RAW, new byte[]{2}).and().send(); - reader.receive(); - Assert.assertEquals(2, meter.value(RECEIVED)); - Assert.assertEquals(2, meter.value(DELIVERED)); - Assert.assertEquals(55, meter.value(BYTES)); - Assert.assertEquals(100, meter.collect(CREDIT)); - - retry.closeStream(Status.of(StatusCode.UNAUTHORIZED)); - Assert.assertEquals(2, meter.value(ERRORS)); - meter.assertAttribute(ERRORS, "retry_decision", "stop"); - meter.assertAttribute(ERRORS, "status_code", "UNAUTHORIZED"); - meter.assertAttribute(ERRORS, "error.type", "ydb_error"); - meter.assertAttribute(ERRORS, "consumer", "consumer"); - meter.assertAttribute(ERRORS, "reader.name", "reader"); - meter.assertNoGauges(); - } finally { - reader.shutdown(); - } + public void gaugesObservePartitionSessionsAndProtocolCredit() throws InterruptedException { + String partitions = "ydb.topic.reader.partition_session.count"; + String credit = "ydb.topic.reader.credit_balance_bytes"; + RecordingMeter meter = new RecordingMeter(); + ReadStreamMock stream = new ReadStreamMock(); + TopicRpc rpc = Mockito.mock(TopicRpc.class); + Mockito.when(rpc.getScheduler()).thenReturn(Mockito.mock(ScheduledExecutorService.class)); + Mockito.when(rpc.readSession(Mockito.anyString())).thenReturn(stream); + TopicClient client = TopicClientImpl.newClient(rpc).build(); + SyncReader reader = client.createSyncReader(ReaderSettings.newBuilder() + .addTopic("/topic").setConsumerName("consumer") + .setMaxMemoryUsageBytes(100).withMeter(meter, "reader").build()); + try { + Assert.assertTrue(meter.gauges.isEmpty()); + reader.init(); + Assert.assertTrue(meter.gauges.isEmpty()); + stream.responseInit("session"); + Assert.assertEquals(0, meter.collect(partitions)); + Assert.assertEquals(100, meter.collect(credit)); + stream.responseStartPartition("/topic", 42, 0); + Assert.assertEquals(1, meter.collect(partitions)); + Assert.assertEquals(100, meter.collect(credit)); + stream.responseData(20).partition(1, 0).batch(Codec.RAW, new byte[]{1}).and().send(); + Assert.assertEquals(80, meter.collect(credit)); + Assert.assertNotNull(reader.receive(1, TimeUnit.SECONDS)); + Assert.assertEquals(100, meter.collect(credit)); + stream.closeStream(Status.of(StatusCode.OVERLOADED)); + Assert.assertTrue(meter.gauges.isEmpty()); + } finally { + reader.shutdown(); + client.close(); } - Assert.assertEquals(2, meter.value(ERRORS)); - meter.assertNoGauges(); + Assert.assertTrue(meter.gauges.isEmpty()); } - private static TopicRpc mockRpc(ReadStreamMock first, ReadStreamMock... retries) { - TopicRpc rpc = Mockito.mock(TopicRpc.class); - Mockito.when(rpc.getScheduler()).thenReturn(Mockito.mock(ScheduledExecutorService.class)); - Mockito.when(rpc.readSession(Mockito.anyString())).thenReturn(first, retries); - return rpc; + private static class RecordingMeter implements Meter { + private final Map counters = new ConcurrentHashMap<>(); + private final Map attributes = new ConcurrentHashMap<>(); + private final Map>> gauges = new ConcurrentHashMap<>(); + + @Override + public MetricRegistration registerLongGauge( + String name, String unit, String description, Consumer callback) { + List> callbacks = gauges.computeIfAbsent(name, key -> new ArrayList<>()); + callbacks.add(callback); + return () -> { + callbacks.remove(callback); + if (callbacks.isEmpty()) { + gauges.remove(name, callbacks); + } + }; + } + + long collect(String name) { + Long[] value = new Long[1]; + Assert.assertEquals(1, gauges.get(name).size()); + gauges.get(name).get(0).accept((observed, attrs) -> value[0] = observed); + return value[0]; + } + + @Override + public LongCounter createCounter(String name, String unit, String description) { + AtomicLong counter = counters.computeIfAbsent(name, key -> new AtomicLong()); + return (value, attrs) -> { + counter.addAndGet(value); + attributes.put(name, attrs); + }; + } + + long value(String name) { + AtomicLong counter = counters.get(name); + return counter == null ? 0 : counter.get(); + } + + void assertAttribute(String name, String key, String value) { + Assert.assertTrue(Arrays.stream(attributes.get(name)) + .anyMatch(attr -> attr.getKey().equals(key) && attr.getValue().equals(value))); + } } } diff --git a/topic/src/test/java/tech/ydb/topic/utils/TopicMeterMock.java b/topic/src/test/java/tech/ydb/topic/utils/TopicMeterMock.java deleted file mode 100644 index 52a62263d..000000000 --- a/topic/src/test/java/tech/ydb/topic/utils/TopicMeterMock.java +++ /dev/null @@ -1,67 +0,0 @@ -package tech.ydb.topic.utils; - -import java.util.Arrays; -import java.util.Map; -import java.util.concurrent.ConcurrentHashMap; -import java.util.concurrent.atomic.AtomicLong; -import java.util.function.Consumer; - -import org.junit.Assert; - -import tech.ydb.core.metrics.Attr; -import tech.ydb.core.metrics.LongCounter; -import tech.ydb.core.metrics.LongMeasurement; -import tech.ydb.core.metrics.Meter; -import tech.ydb.core.metrics.MetricRegistration; - -public class TopicMeterMock implements Meter { - private final Map counters = new ConcurrentHashMap<>(); - private final Map attributes = new ConcurrentHashMap<>(); - private final Map> gauges = new ConcurrentHashMap<>(); - - @Override - public LongCounter createCounter(String name, String unit, String description) { - AtomicLong counter = new AtomicLong(); - Assert.assertNull("Counter registered twice: " + name, counters.putIfAbsent(name, counter)); - return (value, attrs) -> { - attributes.put(name, attrs); - counter.addAndGet(value); - }; - } - - @Override - public MetricRegistration registerLongGauge( - String name, String unit, String description, Consumer callback) { - Assert.assertNull("Gauge registered twice: " + name, gauges.putIfAbsent(name, callback)); - return () -> gauges.remove(name, callback); - } - - public long value(String name) { - AtomicLong counter = counters.get(name); - Assert.assertNotNull("Counter not registered: " + name, counter); - return counter.get(); - } - - public long collect(String name) { - Consumer callback = gauges.get(name); - Assert.assertNotNull("Gauge not registered: " + name, callback); - Long[] value = new Long[1]; - callback.accept((observed, attrs) -> { - attributes.put(name, attrs); - value[0] = observed; - }); - Assert.assertNotNull("Gauge did not record: " + name, value[0]); - return value[0]; - } - - public void assertAttribute(String name, String key, String value) { - Attr[] attrs = attributes.get(name); - Assert.assertNotNull("Metric has no attributes: " + name, attrs); - Assert.assertTrue(name + ": missing " + key + "=" + value, - Arrays.stream(attrs).anyMatch(attr -> attr.getKey().equals(key) && attr.getValue().equals(value))); - } - - public void assertNoGauges() { - Assert.assertTrue("Gauges still registered: " + gauges.keySet(), gauges.isEmpty()); - } -} diff --git a/topic/src/test/java/tech/ydb/topic/write/impl/WriterMetricsTest.java b/topic/src/test/java/tech/ydb/topic/write/impl/WriterMetricsTest.java index 70329498c..1a4e7b03b 100644 --- a/topic/src/test/java/tech/ydb/topic/write/impl/WriterMetricsTest.java +++ b/topic/src/test/java/tech/ydb/topic/write/impl/WriterMetricsTest.java @@ -1,39 +1,44 @@ package tech.ydb.topic.write.impl; -import java.util.concurrent.CompletableFuture; +import java.util.HashMap; +import java.util.Map; import java.util.concurrent.ScheduledExecutorService; +import java.util.function.Consumer; +import java.util.function.LongSupplier; import org.junit.Assert; import org.junit.Test; import org.mockito.Mockito; import tech.ydb.core.Status; -import tech.ydb.core.StatusCode; +import tech.ydb.core.metrics.LongCounter; +import tech.ydb.core.metrics.LongMeasurement; +import tech.ydb.core.metrics.Meter; +import tech.ydb.core.metrics.MetricRegistration; import tech.ydb.topic.TopicClient; import tech.ydb.topic.TopicRpc; import tech.ydb.topic.description.Codec; import tech.ydb.topic.impl.TopicClientImpl; import tech.ydb.topic.settings.WriterSettings; -import tech.ydb.topic.utils.TopicMeterMock; import tech.ydb.topic.write.AsyncWriter; import tech.ydb.topic.write.Message; import tech.ydb.topic.write.QueueOverflowException; -import tech.ydb.topic.write.WriteAck; public class WriterMetricsTest { - private static final String SENDING = "ydb.topic.writer.sending.messages"; - private static final String BYTES = "ydb.topic.writer.sending.bytes"; - private static final String WRITTEN = "ydb.topic.writer.written.messages"; - private static final String ERRORS = "ydb.topic.writer.session.errors"; - private static final String USED = "ydb.topic.writer.buffer.used.bytes"; - private static final String LIMIT = "ydb.topic.writer.buffer.limit.bytes"; @Test - public void writerSendsMessagesAndShutsDown() throws QueueOverflowException { - TopicMeterMock meter = new TopicMeterMock(); + public void writerCountersCountAcceptedMessagesAndAcknowledgements() throws QueueOverflowException { + Map counters = new HashMap<>(); + Meter meter = new Meter() { + @Override + public LongCounter createCounter(String name, String unit, String description) { + counters.put(name, 0L); + return (value, attrs) -> counters.put(name, counters.get(name) + value); + } + }; WriteStreamMock stream = new WriteStreamMock(); WriterSettings settings = WriterSettings.newBuilder() - .setTopicPath("/test/topic").setCodec(Codec.RAW).setMaxSendBufferMemorySize(100) + .setTopicPath("/test/topic").setCodec(Codec.RAW) .withMeter(meter, "writer").build(); try (TopicClient client = TopicClientImpl.newClient(mockRpc(stream)) .setCompressionExecutor(Runnable::run).build()) { @@ -41,106 +46,67 @@ public void writerSendsMessagesAndShutsDown() throws QueueOverflowException { try { writer.init(); stream.sendInitResponse(0); - Assert.assertEquals(0, meter.value(SENDING)); - Assert.assertEquals(0, meter.value(WRITTEN)); - Assert.assertEquals(0, meter.value(ERRORS)); - Assert.assertEquals(0, meter.collect(USED)); - Assert.assertEquals(100, meter.collect(LIMIT)); - - CompletableFuture first = writer.send(Message.of(new byte[]{1, 2})); - CompletableFuture second = writer.send(Message.of(new byte[]{3, 4, 5})); - Assert.assertFalse(first.isDone()); - Assert.assertFalse(second.isDone()); - Assert.assertEquals(2, meter.value(SENDING)); - Assert.assertEquals(5, meter.value(BYTES)); - Assert.assertEquals(0, meter.value(WRITTEN)); - Assert.assertEquals(5, meter.collect(USED)); - Assert.assertEquals(100, meter.collect(LIMIT)); - + writer.send(Message.of(new byte[]{1, 2})); + writer.send(Message.of(new byte[]{3, 4, 5})); stream.sendAckResponse(2, 10); - Assert.assertTrue(first.isDone()); - Assert.assertTrue(second.isDone()); - Assert.assertEquals(2, meter.value(SENDING)); - Assert.assertEquals(5, meter.value(BYTES)); - Assert.assertEquals(2, meter.value(WRITTEN)); - Assert.assertEquals(0, meter.value(ERRORS)); - Assert.assertEquals(0, meter.collect(USED)); - meter.assertAttribute(SENDING, "topic", "/test/topic"); - meter.assertAttribute(BYTES, "writer.name", "writer"); - meter.assertAttribute(WRITTEN, "writer.name", "writer"); - meter.assertAttribute(USED, "topic", "/test/topic"); - meter.assertAttribute(LIMIT, "writer.name", "writer"); + Assert.assertEquals(Long.valueOf(2), counters.get("ydb.topic.writer.sending.messages")); + Assert.assertEquals(Long.valueOf(5), counters.get("ydb.topic.writer.sending.bytes")); + Assert.assertEquals(Long.valueOf(2), counters.get("ydb.topic.writer.written.messages")); } finally { writer.shutdown(); stream.close(Status.SUCCESS); } } - Assert.assertEquals(0, meter.value(ERRORS)); - meter.assertNoGauges(); } @Test - public void writerReconnectsThenStopsOnTerminalError() throws QueueOverflowException { - TopicMeterMock meter = new TopicMeterMock(); - WriteStreamMock first = new WriteStreamMock(); - WriteStreamMock retry = new WriteStreamMock(); + public void writerBufferGaugesTrackBudgetAndLimit() throws QueueOverflowException { + Map gauges = new HashMap<>(); + Meter meter = new Meter() { + @Override + public MetricRegistration registerLongGauge(String name, String unit, String description, + Consumer callback) { + gauges.put(name, () -> { + long[] value = new long[1]; + callback.accept((observed, attrs) -> value[0] = observed); + return value[0]; + }); + return () -> gauges.remove(name); + } + }; + String used = "ydb.topic.writer.buffer.used.bytes"; + String limit = "ydb.topic.writer.buffer.limit.bytes"; + WriteStreamMock stream = new WriteStreamMock(); WriterSettings settings = WriterSettings.newBuilder() .setTopicPath("/test/topic").setCodec(Codec.RAW).setMaxSendBufferMemorySize(100) - .setRetryConfig(status -> status.getCode() == StatusCode.TRANSPORT_UNAVAILABLE - ? (attempt, elapsed) -> 0 : null) .withMeter(meter, "writer").build(); - try (TopicClient client = TopicClientImpl.newClient(mockRpc(first, retry)) + try (TopicClient client = TopicClientImpl.newClient(mockRpc(stream)) .setCompressionExecutor(Runnable::run).build()) { AsyncWriter writer = client.createAsyncWriter(settings); try { writer.init(); - first.sendInitResponse(0); - CompletableFuture written = writer.send(Message.of(new byte[]{1, 2})); - CompletableFuture pending = writer.send(Message.of(new byte[]{3, 4, 5})); - first.close(Status.of(StatusCode.TRANSPORT_UNAVAILABLE)); - Assert.assertEquals(1, meter.value(ERRORS)); - meter.assertAttribute(ERRORS, "retry_decision", "retry"); - meter.assertAttribute(ERRORS, "status_code", "TRANSPORT_UNAVAILABLE"); - meter.assertAttribute(ERRORS, "error.type", "transport_error"); - Assert.assertEquals(2, meter.value(SENDING)); - Assert.assertEquals(5, meter.value(BYTES)); - Assert.assertEquals(0, meter.value(WRITTEN)); - Assert.assertEquals(5, meter.collect(USED)); - Assert.assertEquals(100, meter.collect(LIMIT)); - - retry.sendInitResponse(0); - retry.sendAckResponse(1, 10); - Assert.assertTrue(written.isDone()); - Assert.assertFalse(pending.isDone()); - Assert.assertEquals(2, meter.value(SENDING)); - Assert.assertEquals(5, meter.value(BYTES)); - Assert.assertEquals(1, meter.value(WRITTEN)); - Assert.assertEquals(3, meter.collect(USED)); - - retry.close(Status.of(StatusCode.UNAUTHORIZED)); - Assert.assertTrue(pending.isCompletedExceptionally()); - Assert.assertEquals(2, meter.value(ERRORS)); - Assert.assertEquals(2, meter.value(SENDING)); - Assert.assertEquals(5, meter.value(BYTES)); - Assert.assertEquals(1, meter.value(WRITTEN)); - meter.assertAttribute(ERRORS, "retry_decision", "stop"); - meter.assertAttribute(ERRORS, "status_code", "UNAUTHORIZED"); - meter.assertAttribute(ERRORS, "error.type", "ydb_error"); - meter.assertAttribute(ERRORS, "topic", "/test/topic"); - meter.assertAttribute(ERRORS, "writer.name", "writer"); - meter.assertNoGauges(); + stream.sendInitResponse(0); + Assert.assertEquals(0, gauges.get(used).getAsLong()); + Assert.assertEquals(100, gauges.get(limit).getAsLong()); + writer.send(Message.of(new byte[]{1, 2})); + writer.send(Message.of(new byte[]{3, 4, 5})); + Assert.assertEquals(5, gauges.get(used).getAsLong()); + Assert.assertEquals(100, gauges.get(limit).getAsLong()); + stream.sendAckResponse(2, 10); + Assert.assertEquals(0, gauges.get(used).getAsLong()); } finally { writer.shutdown(); + stream.close(Status.SUCCESS); } } - Assert.assertEquals(2, meter.value(ERRORS)); - meter.assertNoGauges(); + Assert.assertTrue(gauges.isEmpty()); } - private static TopicRpc mockRpc(WriteStreamMock first, WriteStreamMock... retries) { + private static TopicRpc mockRpc(WriteStreamMock stream) { TopicRpc rpc = Mockito.mock(TopicRpc.class); Mockito.when(rpc.getScheduler()).thenReturn(Mockito.mock(ScheduledExecutorService.class)); - Mockito.when(rpc.writeSession(Mockito.anyString())).thenReturn(first, retries); + Mockito.when(rpc.writeSession(Mockito.any(String.class))).thenReturn(stream); return rpc; } + } From 181e39c483c4be1fc607149a420f25374993df29 Mon Sep 17 00:00:00 2001 From: KirillKurdyukov Date: Fri, 16 Oct 2026 00:33:23 +0300 Subject: [PATCH 5/9] Simplify Reader session error classification --- .../ydb/topic/impl/TopicRetryableStream.java | 33 +++++-------------- .../java/tech/ydb/topic/impl/TopicStream.java | 4 --- .../tech/ydb/topic/impl/TopicStreamBase.java | 33 ++++--------------- .../ydb/topic/read/impl/AsyncReaderImpl.java | 3 +- .../tech/ydb/topic/read/impl/ReadSession.java | 4 +-- .../tech/ydb/topic/read/impl/ReaderImpl.java | 20 +++-------- .../ydb/topic/read/impl/ReaderMetrics.java | 4 +-- 7 files changed, 23 insertions(+), 78 deletions(-) diff --git a/topic/src/main/java/tech/ydb/topic/impl/TopicRetryableStream.java b/topic/src/main/java/tech/ydb/topic/impl/TopicRetryableStream.java index f78317ecf..c8ac9c174 100644 --- a/topic/src/main/java/tech/ydb/topic/impl/TopicRetryableStream.java +++ b/topic/src/main/java/tech/ydb/topic/impl/TopicRetryableStream.java @@ -60,14 +60,6 @@ public TopicRetryableStream(Logger logger, String debugId, RetryConfig config, S */ protected abstract void onClose(@Nullable S stream, Status status); - protected void onRetry(@Nullable S stream, Status status, String failureReason) { - onRetry(stream, status); - } - - protected void onClose(@Nullable S stream, Status status, String failureReason) { - onClose(stream, status); - } - public void start() { if (isClosed) { logger.warn("[{}] double start of closed stream, ignored", debugId); @@ -99,14 +91,10 @@ public boolean isClosed() { } public void fail(Status status) { - fail(status, null); - } - - public void fail(Status status, String failureReason) { State local = state.getAndSet(null); if (local != null) { logger.warn("[{}] failed by application-side error {}", debugId, status); - onStreamStop(local.closeStream(), status, retryConfig.getStatusRetryPolicy(status), failureReason); + onStreamStop(local.closeStream(), status, retryConfig.getStatusRetryPolicy(status)); } } @@ -132,19 +120,15 @@ public boolean close() { } private void onStreamStop(S closed, Status status, RetryPolicy policy) { - onStreamStop(closed, status, policy, null); - } - - private void onStreamStop(S closed, Status status, RetryPolicy policy, String failureReason) { if (isClosed) { // stream was already closed (usually with success) - onClose(closed, status, failureReason); + onClose(closed, status); return; } if (policy == null) { logger.warn("[{}] stopped by non-retryable status {}", debugId, status); isClosed = true; - onClose(closed, status, failureReason); + onClose(closed, status); return; } @@ -153,27 +137,27 @@ private void onStreamStop(S closed, Status status, RetryPolicy policy, String fa if (nextRetryMs < 0) { logger.warn("[{}] stopped after retry policy evaluation for status {}", debugId, status); isClosed = true; - onClose(closed, status, failureReason); + onClose(closed, status); return; } if (nextRetryMs == 0) { // retry immediately logger.warn("[{}] retry #{}. Retry immediately...", debugId, retryState.retryNumber()); - onRetry(closed, status, failureReason); + onRetry(closed, status); start(); return; } // retry scheduling logger.warn("[{}] retry #{}. Scheduling reconnect in {}ms...", debugId, retryState.retryNumber(), nextRetryMs); - onRetry(closed, status, failureReason); + onRetry(closed, status); try { scheduler.schedule(this::start, nextRetryMs, TimeUnit.MILLISECONDS); } catch (Exception ex) { logger.error("[{}] cannot schedule reconnect, stopping", debugId, ex); isClosed = true; - onClose(closed, status, failureReason); + onClose(closed, status); } } @@ -227,8 +211,7 @@ private void startStream(S local) { } if (status != null) { - onStreamStop(local, status, retryConfig.getStatusRetryPolicy(status), - local.getFailureReason(status)); + onStreamStop(local, status, retryConfig.getStatusRetryPolicy(status)); } else { Status wrapped = Status.of(StatusCode.CLIENT_INTERNAL_ERROR, th); onStreamStop(local, wrapped, retryConfig.getThrowableRetryPolicy(th)); diff --git a/topic/src/main/java/tech/ydb/topic/impl/TopicStream.java b/topic/src/main/java/tech/ydb/topic/impl/TopicStream.java index 856065601..75236073b 100644 --- a/topic/src/main/java/tech/ydb/topic/impl/TopicStream.java +++ b/topic/src/main/java/tech/ydb/topic/impl/TopicStream.java @@ -10,10 +10,6 @@ public interface TopicStream { CompletableFuture start(Consumer messageHandler); - default String getFailureReason(Status status) { - return null; - } - void send(W request); void close(); } diff --git a/topic/src/main/java/tech/ydb/topic/impl/TopicStreamBase.java b/topic/src/main/java/tech/ydb/topic/impl/TopicStreamBase.java index 4dc4cdd18..522143003 100644 --- a/topic/src/main/java/tech/ydb/topic/impl/TopicStreamBase.java +++ b/topic/src/main/java/tech/ydb/topic/impl/TopicStreamBase.java @@ -16,8 +16,7 @@ public abstract class TopicStreamBase impl private final String debugId; private final GrpcReadWriteStream stream; private final W initRequest; - private final CompletableFuture completion = new CompletableFuture<>(); - private final CompletableFuture streamStatus = completion.thenApply(value -> value.status); + private final CompletableFuture streamStatus = new CompletableFuture<>(); private volatile String token; private volatile boolean isStopped = false; @@ -41,7 +40,7 @@ public CompletableFuture start(Consumer messageHandler) { messageHandler.accept(msg); } else { logger.warn("[{}] stopped by getting status {}", debugId, messageStatus); - if (completion.complete(new Completion(messageStatus, null))) { + if (streamStatus.complete(messageStatus)) { stream.close(); } } @@ -49,10 +48,10 @@ public CompletableFuture start(Consumer messageHandler) { isStopped = true; Status status = st != null ? st : Status.of(StatusCode.CLIENT_INTERNAL_ERROR, th); logger.debug("[{}] finished with status {}", debugId, status); - completion.complete(new Completion(status, null)); + streamStatus.complete(status); }); - if (!completion.isDone()) { + if (!streamStatus.isDone()) { stream.sendNext(initRequest); } @@ -62,30 +61,20 @@ public CompletableFuture start(Consumer messageHandler) { @Override public void close() { logger.debug("[{}] closed by app", debugId); - if (!completion.isDone()) { + if (!streamStatus.isDone()) { isStopped = true; stream.close(); } } protected void fail(Status status) { - fail(status, null); - } - - protected void fail(Status status, String reason) { logger.warn("[{}] stopped by fail {}", debugId, status); - if (completion.complete(new Completion(status, reason))) { + if (streamStatus.complete(status)) { isStopped = true; stream.close(); } } - @Override - public String getFailureReason(Status status) { - Completion value = completion.getNow(null); - return value != null && value.status == status ? value.reason : null; - } - @Override public void send(W req) { if (isStopped) { @@ -103,14 +92,4 @@ public void send(W req) { stream.sendNext(req); } - - private static final class Completion { - private final Status status; - private final String reason; - - Completion(Status status, String reason) { - this.status = status; - this.reason = reason; - } - } } diff --git a/topic/src/main/java/tech/ydb/topic/read/impl/AsyncReaderImpl.java b/topic/src/main/java/tech/ydb/topic/read/impl/AsyncReaderImpl.java index 1b523bbbf..352847cfa 100644 --- a/topic/src/main/java/tech/ydb/topic/read/impl/AsyncReaderImpl.java +++ b/topic/src/main/java/tech/ydb/topic/read/impl/AsyncReaderImpl.java @@ -119,8 +119,7 @@ private void close(Status status) { private void failSession(Throwable th, String callbackName) { String errorMessage = "Unhandled throwable in " + callbackName + " user callback: " + th.getMessage(); logger.error(errorMessage, th); - impl.fail(Status.of(StatusCode.CLIENT_INTERNAL_ERROR, th, Issue.of(errorMessage, Issue.Severity.ERROR)), - "user_callback_error"); + impl.fail(Status.of(StatusCode.CLIENT_INTERNAL_ERROR, th, Issue.of(errorMessage, Issue.Severity.ERROR))); } private class Impl extends ReaderImpl { diff --git a/topic/src/main/java/tech/ydb/topic/read/impl/ReadSession.java b/topic/src/main/java/tech/ydb/topic/read/impl/ReadSession.java index 2f2ec0038..6b26f3aa1 100644 --- a/topic/src/main/java/tech/ydb/topic/read/impl/ReadSession.java +++ b/topic/src/main/java/tech/ydb/topic/read/impl/ReadSession.java @@ -209,7 +209,7 @@ public void onStartPartition(YdbTopic.StreamReadMessage.StartPartitionSessionReq logger.error("[{}] Received second StartPartitionSessionRequest for the already active {}", debugId, ps); Issue issue = Issue.of("Restarting read session due to receiving second StartPartitionSessionRequest with " + ps, Issue.Severity.FATAL); - fail(Status.of(StatusCode.CLIENT_INTERNAL_ERROR, issue), "protocol_error"); + fail(Status.of(StatusCode.CLIENT_INTERNAL_ERROR, issue)); return; } @@ -267,7 +267,7 @@ public void onStopPartition(YdbTopic.StreamReadMessage.StopPartitionSessionReque "but have no such partition session active", debugId, psid); Issue issue = Issue.of("Restarting read session due to receiving StopPartitionSessionRequest with " + "PartitionSessionId " + psid + " that SDK knows nothing about", Issue.Severity.FATAL); - fail(Status.of(StatusCode.CLIENT_INTERNAL_ERROR, issue), "protocol_error"); + fail(Status.of(StatusCode.CLIENT_INTERNAL_ERROR, issue)); } public void onRead(YdbTopic.StreamReadMessage.ReadResponse response) { diff --git a/topic/src/main/java/tech/ydb/topic/read/impl/ReaderImpl.java b/topic/src/main/java/tech/ydb/topic/read/impl/ReaderImpl.java index a970cecfb..e643a85e3 100644 --- a/topic/src/main/java/tech/ydb/topic/read/impl/ReaderImpl.java +++ b/topic/src/main/java/tech/ydb/topic/read/impl/ReaderImpl.java @@ -67,12 +67,7 @@ protected CompletableFuture> createNewStream(String id) { @Override protected void onRetry(ReadSession stream, Status status) { - onRetry(stream, status, null); - } - - @Override - protected void onRetry(ReadSession stream, Status status, String failureReason) { - config.getMetrics().reportSessionError(status, true, failureReason); + config.getMetrics().reportSessionError(status, true); logger.warn("[{}] stopped by status {}", debugId, status); currentSessionId = null; if (errorHandler != null) { @@ -89,14 +84,9 @@ protected void onRetry(ReadSession stream, Status status, String failureReason) @Override protected void onClose(ReadSession stream, Status status) { - onClose(stream, status, null); - } - - @Override - protected void onClose(ReadSession stream, Status status, String failureReason) { currentSessionId = null; if (!status.isSuccess()) { - config.getMetrics().reportSessionError(status, false, failureReason); + config.getMetrics().reportSessionError(status, false); logger.warn("[{}] closed by status {}", debugId, status); } else { logger.info("[{}] closed by status {}", debugId, status); @@ -190,8 +180,7 @@ public CompletableFuture updateOffsetsInTransaction(YdbTransaction trans " with partition offsets from read session " + debugId + " was not committed with status: " + status; if (sessionId == currentSessionId) { - fail(Status.of(StatusCode.CLIENT_INTERNAL_ERROR, Issue.of(msg, Issue.Severity.ERROR)), - "transaction_error"); + fail(Status.of(StatusCode.CLIENT_INTERNAL_ERROR, Issue.of(msg, Issue.Severity.ERROR))); } } if (error != null) { @@ -199,8 +188,7 @@ public CompletableFuture updateOffsetsInTransaction(YdbTransaction trans " with partition offsets from read session " + debugId + " was not committed with reason: " + error.getMessage(); if (sessionId == currentSessionId) { - fail(Status.of(StatusCode.CLIENT_INTERNAL_ERROR, Issue.of(msg, Issue.Severity.ERROR)), - "transaction_error"); + fail(Status.of(StatusCode.CLIENT_INTERNAL_ERROR, Issue.of(msg, Issue.Severity.ERROR))); } } }); diff --git a/topic/src/main/java/tech/ydb/topic/read/impl/ReaderMetrics.java b/topic/src/main/java/tech/ydb/topic/read/impl/ReaderMetrics.java index c61a780fe..3ccd4d520 100644 --- a/topic/src/main/java/tech/ydb/topic/read/impl/ReaderMetrics.java +++ b/topic/src/main/java/tech/ydb/topic/read/impl/ReaderMetrics.java @@ -73,12 +73,12 @@ void reportReceivedBytes(long bytes) { } } - void reportSessionError(Status status, boolean retry, String failureReason) { + void reportSessionError(Status status, boolean retry) { StatusCode code = status.getCode(); Attr[] attributes = Arrays.copyOf(commonAttributes, commonAttributes.length + 3); attributes[commonAttributes.length] = Attr.of("retry_decision", retry ? "retry" : "stop"); attributes[commonAttributes.length + 1] = Attr.of("status_code", code.name()); - String errorType = failureReason != null ? failureReason + String errorType = code == StatusCode.CLIENT_INTERNAL_ERROR ? "client_error" : code == StatusCode.SUCCESS || code == StatusCode.UNUSED_STATUS ? "session_closed" : code.isTransportError() ? "transport_error" : "ydb_error"; attributes[commonAttributes.length + 2] = Attr.of("error.type", errorType); From f448302f5c547627f7397f1d7bf618cb41b72fa6 Mon Sep 17 00:00:00 2001 From: KirillKurdyukov Date: Fri, 16 Oct 2026 01:04:00 +0300 Subject: [PATCH 6/9] Restore Topic session error mock tests --- .../topic/read/impl/ReaderMetricsTest.java | 245 ++++++++---------- .../tech/ydb/topic/utils/TopicMeterMock.java | 67 +++++ .../topic/write/impl/WriterMetricsTest.java | 144 ++++++---- 3 files changed, 267 insertions(+), 189 deletions(-) create mode 100644 topic/src/test/java/tech/ydb/topic/utils/TopicMeterMock.java diff --git a/topic/src/test/java/tech/ydb/topic/read/impl/ReaderMetricsTest.java b/topic/src/test/java/tech/ydb/topic/read/impl/ReaderMetricsTest.java index c300e764d..61d3a8ac0 100644 --- a/topic/src/test/java/tech/ydb/topic/read/impl/ReaderMetricsTest.java +++ b/topic/src/test/java/tech/ydb/topic/read/impl/ReaderMetricsTest.java @@ -1,14 +1,6 @@ package tech.ydb.topic.read.impl; -import java.util.ArrayList; -import java.util.Arrays; -import java.util.List; -import java.util.Map; -import java.util.concurrent.ConcurrentHashMap; import java.util.concurrent.ScheduledExecutorService; -import java.util.concurrent.TimeUnit; -import java.util.concurrent.atomic.AtomicLong; -import java.util.function.Consumer; import org.junit.Assert; import org.junit.Test; @@ -16,149 +8,134 @@ import tech.ydb.core.Status; import tech.ydb.core.StatusCode; -import tech.ydb.core.metrics.Attr; -import tech.ydb.core.metrics.LongCounter; -import tech.ydb.core.metrics.LongMeasurement; -import tech.ydb.core.metrics.Meter; -import tech.ydb.core.metrics.MetricRegistration; import tech.ydb.topic.TopicClient; import tech.ydb.topic.TopicRpc; import tech.ydb.topic.description.Codec; import tech.ydb.topic.impl.TopicClientImpl; import tech.ydb.topic.read.SyncReader; import tech.ydb.topic.settings.ReaderSettings; -import tech.ydb.topic.settings.TopicReadSettings; +import tech.ydb.topic.utils.TopicMeterMock; public class ReaderMetricsTest { + private static final String DELIVERED = "ydb.topic.reader.delivered.messages"; + private static final String RECEIVED = "ydb.topic.reader.received.messages"; + private static final String BYTES = "ydb.topic.reader.received.bytes"; + private static final String ERRORS = "ydb.topic.reader.session.errors"; + private static final String PARTITIONS = "ydb.topic.reader.partition_session.count"; + private static final String CREDIT = "ydb.topic.reader.credit_balance_bytes"; @Test - public void readerCountersIncrementOnReceive() throws InterruptedException { - String delivered = "ydb.topic.reader.delivered.messages"; - String receivedMessages = "ydb.topic.reader.received.messages"; - String receivedBytes = "ydb.topic.reader.received.bytes"; - RecordingMeter meter = new RecordingMeter(); + public void readerReceivesMessagesAndShutsDown() throws InterruptedException { + TopicMeterMock meter = new TopicMeterMock(); ReadStreamMock stream = new ReadStreamMock(); - TopicRpc rpc = Mockito.mock(TopicRpc.class); - Mockito.when(rpc.getScheduler()).thenReturn(Mockito.mock(ScheduledExecutorService.class)); - Mockito.when(rpc.readSession(Mockito.any(String.class))).thenReturn(stream); - - TopicClient client = TopicClientImpl.newClient(rpc).build(); - SyncReader reader = client.createSyncReader(ReaderSettings.newBuilder() - .addTopic(TopicReadSettings.newBuilder().setPath("/topic").build()) - .setConsumerName("consumer") - .withMeter(meter, "reader") - .build()); - - try { - reader.init(); - stream.assertLastMessage().isInitRequest("consumer", "/topic").hasReaderName("reader"); - stream.responseInit("read-session"); - stream.responseStartPartition("/topic", 42, 0); - byte[] msg1 = new byte[] { 1 }; - byte[] msg2 = new byte[] { 2 }; - stream.responseData(25).partition(1, 0) - .batch(Codec.RAW, msg1, msg2) - .and().send(); - Assert.assertEquals(2, meter.value(receivedMessages)); - Assert.assertEquals(25, meter.value(receivedBytes)); - - stream.responseData(30).partition(1, 2) - .batch(Codec.RAW, msg1, msg2, msg1) - .and().send(); - Assert.assertEquals(5, meter.value(receivedMessages)); - Assert.assertEquals(55, meter.value(receivedBytes)); - - Assert.assertEquals(0, meter.value(delivered)); - meter.assertAttribute(receivedMessages, "topic", "/topic"); - meter.assertAttribute(receivedMessages, "reader.name", "reader"); - - reader.receive(); - reader.receive(); - Assert.assertEquals(2, meter.value(delivered)); - } finally { - reader.shutdown(); - client.close(); + try (TopicClient client = TopicClientImpl.newClient(mockRpc(stream)).build()) { + SyncReader reader = client.createSyncReader(ReaderSettings.newBuilder() + .addTopic("/topic").setConsumerName("consumer") + .setMaxMemoryUsageBytes(100).withMeter(meter, "reader").build()); + try { + meter.assertNoGauges(); + reader.init(); + stream.assertLastMessage().isInitRequest("consumer", "/topic").hasReaderName("reader"); + meter.assertNoGauges(); + stream.responseInit("session"); + Assert.assertEquals(0, meter.collect(PARTITIONS)); + Assert.assertEquals(100, meter.collect(CREDIT)); + stream.responseStartPartition("/topic", 42, 0); + Assert.assertEquals(1, meter.collect(PARTITIONS)); + + byte[] msg1 = new byte[]{1}; + byte[] msg2 = new byte[]{2}; + stream.responseData(25).partition(1, 0).batch(Codec.RAW, msg1, msg2).and().send(); + Assert.assertEquals(2, meter.value(RECEIVED)); + Assert.assertEquals(25, meter.value(BYTES)); + Assert.assertEquals(75, meter.collect(CREDIT)); + Assert.assertEquals(0, meter.value(DELIVERED)); + Assert.assertEquals(0, meter.value(ERRORS)); + + stream.responseData(30).partition(1, 2).batch(Codec.RAW, msg1, msg2, msg1).and().send(); + Assert.assertEquals(5, meter.value(RECEIVED)); + Assert.assertEquals(55, meter.value(BYTES)); + Assert.assertEquals(45, meter.collect(CREDIT)); + reader.receive(); + reader.receive(); + Assert.assertEquals(2, meter.value(DELIVERED)); + reader.receive(); + reader.receive(); + reader.receive(); + Assert.assertEquals(5, meter.value(DELIVERED)); + Assert.assertEquals(100, meter.collect(CREDIT)); + Assert.assertEquals(1, meter.collect(PARTITIONS)); + meter.assertAttribute(RECEIVED, "topic", "/topic"); + meter.assertAttribute(DELIVERED, "topic", "/topic"); + meter.assertAttribute(BYTES, "consumer", "consumer"); + meter.assertAttribute(PARTITIONS, "reader.name", "reader"); + meter.assertAttribute(CREDIT, "consumer", "consumer"); + } finally { + reader.shutdown(); + } } + Assert.assertEquals(0, meter.value(ERRORS)); + meter.assertNoGauges(); } @Test - public void gaugesObservePartitionSessionsAndProtocolCredit() throws InterruptedException { - String partitions = "ydb.topic.reader.partition_session.count"; - String credit = "ydb.topic.reader.credit_balance_bytes"; - RecordingMeter meter = new RecordingMeter(); - ReadStreamMock stream = new ReadStreamMock(); - TopicRpc rpc = Mockito.mock(TopicRpc.class); - Mockito.when(rpc.getScheduler()).thenReturn(Mockito.mock(ScheduledExecutorService.class)); - Mockito.when(rpc.readSession(Mockito.anyString())).thenReturn(stream); - TopicClient client = TopicClientImpl.newClient(rpc).build(); - SyncReader reader = client.createSyncReader(ReaderSettings.newBuilder() - .addTopic("/topic").setConsumerName("consumer") - .setMaxMemoryUsageBytes(100).withMeter(meter, "reader").build()); - try { - Assert.assertTrue(meter.gauges.isEmpty()); - reader.init(); - Assert.assertTrue(meter.gauges.isEmpty()); - stream.responseInit("session"); - Assert.assertEquals(0, meter.collect(partitions)); - Assert.assertEquals(100, meter.collect(credit)); - stream.responseStartPartition("/topic", 42, 0); - Assert.assertEquals(1, meter.collect(partitions)); - Assert.assertEquals(100, meter.collect(credit)); - stream.responseData(20).partition(1, 0).batch(Codec.RAW, new byte[]{1}).and().send(); - Assert.assertEquals(80, meter.collect(credit)); - Assert.assertNotNull(reader.receive(1, TimeUnit.SECONDS)); - Assert.assertEquals(100, meter.collect(credit)); - stream.closeStream(Status.of(StatusCode.OVERLOADED)); - Assert.assertTrue(meter.gauges.isEmpty()); - } finally { - reader.shutdown(); - client.close(); + public void readerReconnectsThenStopsOnTerminalError() throws InterruptedException { + TopicMeterMock meter = new TopicMeterMock(); + ReadStreamMock first = new ReadStreamMock(); + ReadStreamMock retry = new ReadStreamMock(); + try (TopicClient client = TopicClientImpl.newClient(mockRpc(first, retry)).build()) { + SyncReader reader = client.createSyncReader(ReaderSettings.newBuilder() + .addTopic("/topic").setConsumerName("consumer").setMaxMemoryUsageBytes(100) + .setRetryConfig(status -> status.getCode() == StatusCode.TRANSPORT_UNAVAILABLE + ? (attempt, elapsed) -> 0 : null) + .withMeter(meter, "reader").build()); + try { + reader.init(); + first.responseInit("first"); + first.responseStartPartition("/topic", 42, 0); + first.responseData(25).partition(1, 0).batch(Codec.RAW, new byte[]{1}).and().send(); + reader.receive(); + first.closeStream(Status.of(StatusCode.TRANSPORT_UNAVAILABLE)); + Assert.assertEquals(1, meter.value(ERRORS)); + meter.assertAttribute(ERRORS, "retry_decision", "retry"); + meter.assertAttribute(ERRORS, "status_code", "TRANSPORT_UNAVAILABLE"); + meter.assertAttribute(ERRORS, "error.type", "transport_error"); + meter.assertNoGauges(); + Assert.assertEquals(1, meter.value(RECEIVED)); + Assert.assertEquals(1, meter.value(DELIVERED)); + Assert.assertEquals(25, meter.value(BYTES)); + + retry.responseInit("retry"); + retry.responseStartPartition("/topic", 42, 1); + Assert.assertEquals(1, meter.collect(PARTITIONS)); + Assert.assertEquals(100, meter.collect(CREDIT)); + retry.responseData(30).partition(1, 1).batch(Codec.RAW, new byte[]{2}).and().send(); + reader.receive(); + Assert.assertEquals(2, meter.value(RECEIVED)); + Assert.assertEquals(2, meter.value(DELIVERED)); + Assert.assertEquals(55, meter.value(BYTES)); + Assert.assertEquals(100, meter.collect(CREDIT)); + + retry.closeStream(Status.of(StatusCode.UNAUTHORIZED)); + Assert.assertEquals(2, meter.value(ERRORS)); + meter.assertAttribute(ERRORS, "retry_decision", "stop"); + meter.assertAttribute(ERRORS, "status_code", "UNAUTHORIZED"); + meter.assertAttribute(ERRORS, "error.type", "ydb_error"); + meter.assertAttribute(ERRORS, "consumer", "consumer"); + meter.assertAttribute(ERRORS, "reader.name", "reader"); + meter.assertNoGauges(); + } finally { + reader.shutdown(); + } } - Assert.assertTrue(meter.gauges.isEmpty()); + Assert.assertEquals(2, meter.value(ERRORS)); + meter.assertNoGauges(); } - private static class RecordingMeter implements Meter { - private final Map counters = new ConcurrentHashMap<>(); - private final Map attributes = new ConcurrentHashMap<>(); - private final Map>> gauges = new ConcurrentHashMap<>(); - - @Override - public MetricRegistration registerLongGauge( - String name, String unit, String description, Consumer callback) { - List> callbacks = gauges.computeIfAbsent(name, key -> new ArrayList<>()); - callbacks.add(callback); - return () -> { - callbacks.remove(callback); - if (callbacks.isEmpty()) { - gauges.remove(name, callbacks); - } - }; - } - - long collect(String name) { - Long[] value = new Long[1]; - Assert.assertEquals(1, gauges.get(name).size()); - gauges.get(name).get(0).accept((observed, attrs) -> value[0] = observed); - return value[0]; - } - - @Override - public LongCounter createCounter(String name, String unit, String description) { - AtomicLong counter = counters.computeIfAbsent(name, key -> new AtomicLong()); - return (value, attrs) -> { - counter.addAndGet(value); - attributes.put(name, attrs); - }; - } - - long value(String name) { - AtomicLong counter = counters.get(name); - return counter == null ? 0 : counter.get(); - } - - void assertAttribute(String name, String key, String value) { - Assert.assertTrue(Arrays.stream(attributes.get(name)) - .anyMatch(attr -> attr.getKey().equals(key) && attr.getValue().equals(value))); - } + private static TopicRpc mockRpc(ReadStreamMock first, ReadStreamMock... retries) { + TopicRpc rpc = Mockito.mock(TopicRpc.class); + Mockito.when(rpc.getScheduler()).thenReturn(Mockito.mock(ScheduledExecutorService.class)); + Mockito.when(rpc.readSession(Mockito.anyString())).thenReturn(first, retries); + return rpc; } } diff --git a/topic/src/test/java/tech/ydb/topic/utils/TopicMeterMock.java b/topic/src/test/java/tech/ydb/topic/utils/TopicMeterMock.java new file mode 100644 index 000000000..52a62263d --- /dev/null +++ b/topic/src/test/java/tech/ydb/topic/utils/TopicMeterMock.java @@ -0,0 +1,67 @@ +package tech.ydb.topic.utils; + +import java.util.Arrays; +import java.util.Map; +import java.util.concurrent.ConcurrentHashMap; +import java.util.concurrent.atomic.AtomicLong; +import java.util.function.Consumer; + +import org.junit.Assert; + +import tech.ydb.core.metrics.Attr; +import tech.ydb.core.metrics.LongCounter; +import tech.ydb.core.metrics.LongMeasurement; +import tech.ydb.core.metrics.Meter; +import tech.ydb.core.metrics.MetricRegistration; + +public class TopicMeterMock implements Meter { + private final Map counters = new ConcurrentHashMap<>(); + private final Map attributes = new ConcurrentHashMap<>(); + private final Map> gauges = new ConcurrentHashMap<>(); + + @Override + public LongCounter createCounter(String name, String unit, String description) { + AtomicLong counter = new AtomicLong(); + Assert.assertNull("Counter registered twice: " + name, counters.putIfAbsent(name, counter)); + return (value, attrs) -> { + attributes.put(name, attrs); + counter.addAndGet(value); + }; + } + + @Override + public MetricRegistration registerLongGauge( + String name, String unit, String description, Consumer callback) { + Assert.assertNull("Gauge registered twice: " + name, gauges.putIfAbsent(name, callback)); + return () -> gauges.remove(name, callback); + } + + public long value(String name) { + AtomicLong counter = counters.get(name); + Assert.assertNotNull("Counter not registered: " + name, counter); + return counter.get(); + } + + public long collect(String name) { + Consumer callback = gauges.get(name); + Assert.assertNotNull("Gauge not registered: " + name, callback); + Long[] value = new Long[1]; + callback.accept((observed, attrs) -> { + attributes.put(name, attrs); + value[0] = observed; + }); + Assert.assertNotNull("Gauge did not record: " + name, value[0]); + return value[0]; + } + + public void assertAttribute(String name, String key, String value) { + Attr[] attrs = attributes.get(name); + Assert.assertNotNull("Metric has no attributes: " + name, attrs); + Assert.assertTrue(name + ": missing " + key + "=" + value, + Arrays.stream(attrs).anyMatch(attr -> attr.getKey().equals(key) && attr.getValue().equals(value))); + } + + public void assertNoGauges() { + Assert.assertTrue("Gauges still registered: " + gauges.keySet(), gauges.isEmpty()); + } +} diff --git a/topic/src/test/java/tech/ydb/topic/write/impl/WriterMetricsTest.java b/topic/src/test/java/tech/ydb/topic/write/impl/WriterMetricsTest.java index 1a4e7b03b..70329498c 100644 --- a/topic/src/test/java/tech/ydb/topic/write/impl/WriterMetricsTest.java +++ b/topic/src/test/java/tech/ydb/topic/write/impl/WriterMetricsTest.java @@ -1,44 +1,39 @@ package tech.ydb.topic.write.impl; -import java.util.HashMap; -import java.util.Map; +import java.util.concurrent.CompletableFuture; import java.util.concurrent.ScheduledExecutorService; -import java.util.function.Consumer; -import java.util.function.LongSupplier; import org.junit.Assert; import org.junit.Test; import org.mockito.Mockito; import tech.ydb.core.Status; -import tech.ydb.core.metrics.LongCounter; -import tech.ydb.core.metrics.LongMeasurement; -import tech.ydb.core.metrics.Meter; -import tech.ydb.core.metrics.MetricRegistration; +import tech.ydb.core.StatusCode; import tech.ydb.topic.TopicClient; import tech.ydb.topic.TopicRpc; import tech.ydb.topic.description.Codec; import tech.ydb.topic.impl.TopicClientImpl; import tech.ydb.topic.settings.WriterSettings; +import tech.ydb.topic.utils.TopicMeterMock; import tech.ydb.topic.write.AsyncWriter; import tech.ydb.topic.write.Message; import tech.ydb.topic.write.QueueOverflowException; +import tech.ydb.topic.write.WriteAck; public class WriterMetricsTest { + private static final String SENDING = "ydb.topic.writer.sending.messages"; + private static final String BYTES = "ydb.topic.writer.sending.bytes"; + private static final String WRITTEN = "ydb.topic.writer.written.messages"; + private static final String ERRORS = "ydb.topic.writer.session.errors"; + private static final String USED = "ydb.topic.writer.buffer.used.bytes"; + private static final String LIMIT = "ydb.topic.writer.buffer.limit.bytes"; @Test - public void writerCountersCountAcceptedMessagesAndAcknowledgements() throws QueueOverflowException { - Map counters = new HashMap<>(); - Meter meter = new Meter() { - @Override - public LongCounter createCounter(String name, String unit, String description) { - counters.put(name, 0L); - return (value, attrs) -> counters.put(name, counters.get(name) + value); - } - }; + public void writerSendsMessagesAndShutsDown() throws QueueOverflowException { + TopicMeterMock meter = new TopicMeterMock(); WriteStreamMock stream = new WriteStreamMock(); WriterSettings settings = WriterSettings.newBuilder() - .setTopicPath("/test/topic").setCodec(Codec.RAW) + .setTopicPath("/test/topic").setCodec(Codec.RAW).setMaxSendBufferMemorySize(100) .withMeter(meter, "writer").build(); try (TopicClient client = TopicClientImpl.newClient(mockRpc(stream)) .setCompressionExecutor(Runnable::run).build()) { @@ -46,67 +41,106 @@ public LongCounter createCounter(String name, String unit, String description) { try { writer.init(); stream.sendInitResponse(0); - writer.send(Message.of(new byte[]{1, 2})); - writer.send(Message.of(new byte[]{3, 4, 5})); + Assert.assertEquals(0, meter.value(SENDING)); + Assert.assertEquals(0, meter.value(WRITTEN)); + Assert.assertEquals(0, meter.value(ERRORS)); + Assert.assertEquals(0, meter.collect(USED)); + Assert.assertEquals(100, meter.collect(LIMIT)); + + CompletableFuture first = writer.send(Message.of(new byte[]{1, 2})); + CompletableFuture second = writer.send(Message.of(new byte[]{3, 4, 5})); + Assert.assertFalse(first.isDone()); + Assert.assertFalse(second.isDone()); + Assert.assertEquals(2, meter.value(SENDING)); + Assert.assertEquals(5, meter.value(BYTES)); + Assert.assertEquals(0, meter.value(WRITTEN)); + Assert.assertEquals(5, meter.collect(USED)); + Assert.assertEquals(100, meter.collect(LIMIT)); + stream.sendAckResponse(2, 10); - Assert.assertEquals(Long.valueOf(2), counters.get("ydb.topic.writer.sending.messages")); - Assert.assertEquals(Long.valueOf(5), counters.get("ydb.topic.writer.sending.bytes")); - Assert.assertEquals(Long.valueOf(2), counters.get("ydb.topic.writer.written.messages")); + Assert.assertTrue(first.isDone()); + Assert.assertTrue(second.isDone()); + Assert.assertEquals(2, meter.value(SENDING)); + Assert.assertEquals(5, meter.value(BYTES)); + Assert.assertEquals(2, meter.value(WRITTEN)); + Assert.assertEquals(0, meter.value(ERRORS)); + Assert.assertEquals(0, meter.collect(USED)); + meter.assertAttribute(SENDING, "topic", "/test/topic"); + meter.assertAttribute(BYTES, "writer.name", "writer"); + meter.assertAttribute(WRITTEN, "writer.name", "writer"); + meter.assertAttribute(USED, "topic", "/test/topic"); + meter.assertAttribute(LIMIT, "writer.name", "writer"); } finally { writer.shutdown(); stream.close(Status.SUCCESS); } } + Assert.assertEquals(0, meter.value(ERRORS)); + meter.assertNoGauges(); } @Test - public void writerBufferGaugesTrackBudgetAndLimit() throws QueueOverflowException { - Map gauges = new HashMap<>(); - Meter meter = new Meter() { - @Override - public MetricRegistration registerLongGauge(String name, String unit, String description, - Consumer callback) { - gauges.put(name, () -> { - long[] value = new long[1]; - callback.accept((observed, attrs) -> value[0] = observed); - return value[0]; - }); - return () -> gauges.remove(name); - } - }; - String used = "ydb.topic.writer.buffer.used.bytes"; - String limit = "ydb.topic.writer.buffer.limit.bytes"; - WriteStreamMock stream = new WriteStreamMock(); + public void writerReconnectsThenStopsOnTerminalError() throws QueueOverflowException { + TopicMeterMock meter = new TopicMeterMock(); + WriteStreamMock first = new WriteStreamMock(); + WriteStreamMock retry = new WriteStreamMock(); WriterSettings settings = WriterSettings.newBuilder() .setTopicPath("/test/topic").setCodec(Codec.RAW).setMaxSendBufferMemorySize(100) + .setRetryConfig(status -> status.getCode() == StatusCode.TRANSPORT_UNAVAILABLE + ? (attempt, elapsed) -> 0 : null) .withMeter(meter, "writer").build(); - try (TopicClient client = TopicClientImpl.newClient(mockRpc(stream)) + try (TopicClient client = TopicClientImpl.newClient(mockRpc(first, retry)) .setCompressionExecutor(Runnable::run).build()) { AsyncWriter writer = client.createAsyncWriter(settings); try { writer.init(); - stream.sendInitResponse(0); - Assert.assertEquals(0, gauges.get(used).getAsLong()); - Assert.assertEquals(100, gauges.get(limit).getAsLong()); - writer.send(Message.of(new byte[]{1, 2})); - writer.send(Message.of(new byte[]{3, 4, 5})); - Assert.assertEquals(5, gauges.get(used).getAsLong()); - Assert.assertEquals(100, gauges.get(limit).getAsLong()); - stream.sendAckResponse(2, 10); - Assert.assertEquals(0, gauges.get(used).getAsLong()); + first.sendInitResponse(0); + CompletableFuture written = writer.send(Message.of(new byte[]{1, 2})); + CompletableFuture pending = writer.send(Message.of(new byte[]{3, 4, 5})); + first.close(Status.of(StatusCode.TRANSPORT_UNAVAILABLE)); + Assert.assertEquals(1, meter.value(ERRORS)); + meter.assertAttribute(ERRORS, "retry_decision", "retry"); + meter.assertAttribute(ERRORS, "status_code", "TRANSPORT_UNAVAILABLE"); + meter.assertAttribute(ERRORS, "error.type", "transport_error"); + Assert.assertEquals(2, meter.value(SENDING)); + Assert.assertEquals(5, meter.value(BYTES)); + Assert.assertEquals(0, meter.value(WRITTEN)); + Assert.assertEquals(5, meter.collect(USED)); + Assert.assertEquals(100, meter.collect(LIMIT)); + + retry.sendInitResponse(0); + retry.sendAckResponse(1, 10); + Assert.assertTrue(written.isDone()); + Assert.assertFalse(pending.isDone()); + Assert.assertEquals(2, meter.value(SENDING)); + Assert.assertEquals(5, meter.value(BYTES)); + Assert.assertEquals(1, meter.value(WRITTEN)); + Assert.assertEquals(3, meter.collect(USED)); + + retry.close(Status.of(StatusCode.UNAUTHORIZED)); + Assert.assertTrue(pending.isCompletedExceptionally()); + Assert.assertEquals(2, meter.value(ERRORS)); + Assert.assertEquals(2, meter.value(SENDING)); + Assert.assertEquals(5, meter.value(BYTES)); + Assert.assertEquals(1, meter.value(WRITTEN)); + meter.assertAttribute(ERRORS, "retry_decision", "stop"); + meter.assertAttribute(ERRORS, "status_code", "UNAUTHORIZED"); + meter.assertAttribute(ERRORS, "error.type", "ydb_error"); + meter.assertAttribute(ERRORS, "topic", "/test/topic"); + meter.assertAttribute(ERRORS, "writer.name", "writer"); + meter.assertNoGauges(); } finally { writer.shutdown(); - stream.close(Status.SUCCESS); } } - Assert.assertTrue(gauges.isEmpty()); + Assert.assertEquals(2, meter.value(ERRORS)); + meter.assertNoGauges(); } - private static TopicRpc mockRpc(WriteStreamMock stream) { + private static TopicRpc mockRpc(WriteStreamMock first, WriteStreamMock... retries) { TopicRpc rpc = Mockito.mock(TopicRpc.class); Mockito.when(rpc.getScheduler()).thenReturn(Mockito.mock(ScheduledExecutorService.class)); - Mockito.when(rpc.writeSession(Mockito.any(String.class))).thenReturn(stream); + Mockito.when(rpc.writeSession(Mockito.anyString())).thenReturn(first, retries); return rpc; } - } From 103cf839c7182fa6b2bf7f01b4e1cecc4bb85ef9 Mon Sep 17 00:00:00 2001 From: KirillKurdyukov Date: Fri, 16 Oct 2026 01:11:19 +0300 Subject: [PATCH 7/9] Undo cancelled Topic mock test restoration --- .../topic/read/impl/ReaderMetricsTest.java | 245 ++++++++++-------- .../tech/ydb/topic/utils/TopicMeterMock.java | 67 ----- .../topic/write/impl/WriterMetricsTest.java | 144 ++++------ 3 files changed, 189 insertions(+), 267 deletions(-) delete mode 100644 topic/src/test/java/tech/ydb/topic/utils/TopicMeterMock.java diff --git a/topic/src/test/java/tech/ydb/topic/read/impl/ReaderMetricsTest.java b/topic/src/test/java/tech/ydb/topic/read/impl/ReaderMetricsTest.java index 61d3a8ac0..c300e764d 100644 --- a/topic/src/test/java/tech/ydb/topic/read/impl/ReaderMetricsTest.java +++ b/topic/src/test/java/tech/ydb/topic/read/impl/ReaderMetricsTest.java @@ -1,6 +1,14 @@ package tech.ydb.topic.read.impl; +import java.util.ArrayList; +import java.util.Arrays; +import java.util.List; +import java.util.Map; +import java.util.concurrent.ConcurrentHashMap; import java.util.concurrent.ScheduledExecutorService; +import java.util.concurrent.TimeUnit; +import java.util.concurrent.atomic.AtomicLong; +import java.util.function.Consumer; import org.junit.Assert; import org.junit.Test; @@ -8,134 +16,149 @@ import tech.ydb.core.Status; import tech.ydb.core.StatusCode; +import tech.ydb.core.metrics.Attr; +import tech.ydb.core.metrics.LongCounter; +import tech.ydb.core.metrics.LongMeasurement; +import tech.ydb.core.metrics.Meter; +import tech.ydb.core.metrics.MetricRegistration; import tech.ydb.topic.TopicClient; import tech.ydb.topic.TopicRpc; import tech.ydb.topic.description.Codec; import tech.ydb.topic.impl.TopicClientImpl; import tech.ydb.topic.read.SyncReader; import tech.ydb.topic.settings.ReaderSettings; -import tech.ydb.topic.utils.TopicMeterMock; +import tech.ydb.topic.settings.TopicReadSettings; public class ReaderMetricsTest { - private static final String DELIVERED = "ydb.topic.reader.delivered.messages"; - private static final String RECEIVED = "ydb.topic.reader.received.messages"; - private static final String BYTES = "ydb.topic.reader.received.bytes"; - private static final String ERRORS = "ydb.topic.reader.session.errors"; - private static final String PARTITIONS = "ydb.topic.reader.partition_session.count"; - private static final String CREDIT = "ydb.topic.reader.credit_balance_bytes"; @Test - public void readerReceivesMessagesAndShutsDown() throws InterruptedException { - TopicMeterMock meter = new TopicMeterMock(); + public void readerCountersIncrementOnReceive() throws InterruptedException { + String delivered = "ydb.topic.reader.delivered.messages"; + String receivedMessages = "ydb.topic.reader.received.messages"; + String receivedBytes = "ydb.topic.reader.received.bytes"; + RecordingMeter meter = new RecordingMeter(); ReadStreamMock stream = new ReadStreamMock(); - try (TopicClient client = TopicClientImpl.newClient(mockRpc(stream)).build()) { - SyncReader reader = client.createSyncReader(ReaderSettings.newBuilder() - .addTopic("/topic").setConsumerName("consumer") - .setMaxMemoryUsageBytes(100).withMeter(meter, "reader").build()); - try { - meter.assertNoGauges(); - reader.init(); - stream.assertLastMessage().isInitRequest("consumer", "/topic").hasReaderName("reader"); - meter.assertNoGauges(); - stream.responseInit("session"); - Assert.assertEquals(0, meter.collect(PARTITIONS)); - Assert.assertEquals(100, meter.collect(CREDIT)); - stream.responseStartPartition("/topic", 42, 0); - Assert.assertEquals(1, meter.collect(PARTITIONS)); - - byte[] msg1 = new byte[]{1}; - byte[] msg2 = new byte[]{2}; - stream.responseData(25).partition(1, 0).batch(Codec.RAW, msg1, msg2).and().send(); - Assert.assertEquals(2, meter.value(RECEIVED)); - Assert.assertEquals(25, meter.value(BYTES)); - Assert.assertEquals(75, meter.collect(CREDIT)); - Assert.assertEquals(0, meter.value(DELIVERED)); - Assert.assertEquals(0, meter.value(ERRORS)); - - stream.responseData(30).partition(1, 2).batch(Codec.RAW, msg1, msg2, msg1).and().send(); - Assert.assertEquals(5, meter.value(RECEIVED)); - Assert.assertEquals(55, meter.value(BYTES)); - Assert.assertEquals(45, meter.collect(CREDIT)); - reader.receive(); - reader.receive(); - Assert.assertEquals(2, meter.value(DELIVERED)); - reader.receive(); - reader.receive(); - reader.receive(); - Assert.assertEquals(5, meter.value(DELIVERED)); - Assert.assertEquals(100, meter.collect(CREDIT)); - Assert.assertEquals(1, meter.collect(PARTITIONS)); - meter.assertAttribute(RECEIVED, "topic", "/topic"); - meter.assertAttribute(DELIVERED, "topic", "/topic"); - meter.assertAttribute(BYTES, "consumer", "consumer"); - meter.assertAttribute(PARTITIONS, "reader.name", "reader"); - meter.assertAttribute(CREDIT, "consumer", "consumer"); - } finally { - reader.shutdown(); - } + TopicRpc rpc = Mockito.mock(TopicRpc.class); + Mockito.when(rpc.getScheduler()).thenReturn(Mockito.mock(ScheduledExecutorService.class)); + Mockito.when(rpc.readSession(Mockito.any(String.class))).thenReturn(stream); + + TopicClient client = TopicClientImpl.newClient(rpc).build(); + SyncReader reader = client.createSyncReader(ReaderSettings.newBuilder() + .addTopic(TopicReadSettings.newBuilder().setPath("/topic").build()) + .setConsumerName("consumer") + .withMeter(meter, "reader") + .build()); + + try { + reader.init(); + stream.assertLastMessage().isInitRequest("consumer", "/topic").hasReaderName("reader"); + stream.responseInit("read-session"); + stream.responseStartPartition("/topic", 42, 0); + byte[] msg1 = new byte[] { 1 }; + byte[] msg2 = new byte[] { 2 }; + stream.responseData(25).partition(1, 0) + .batch(Codec.RAW, msg1, msg2) + .and().send(); + Assert.assertEquals(2, meter.value(receivedMessages)); + Assert.assertEquals(25, meter.value(receivedBytes)); + + stream.responseData(30).partition(1, 2) + .batch(Codec.RAW, msg1, msg2, msg1) + .and().send(); + Assert.assertEquals(5, meter.value(receivedMessages)); + Assert.assertEquals(55, meter.value(receivedBytes)); + + Assert.assertEquals(0, meter.value(delivered)); + meter.assertAttribute(receivedMessages, "topic", "/topic"); + meter.assertAttribute(receivedMessages, "reader.name", "reader"); + + reader.receive(); + reader.receive(); + Assert.assertEquals(2, meter.value(delivered)); + } finally { + reader.shutdown(); + client.close(); } - Assert.assertEquals(0, meter.value(ERRORS)); - meter.assertNoGauges(); } @Test - public void readerReconnectsThenStopsOnTerminalError() throws InterruptedException { - TopicMeterMock meter = new TopicMeterMock(); - ReadStreamMock first = new ReadStreamMock(); - ReadStreamMock retry = new ReadStreamMock(); - try (TopicClient client = TopicClientImpl.newClient(mockRpc(first, retry)).build()) { - SyncReader reader = client.createSyncReader(ReaderSettings.newBuilder() - .addTopic("/topic").setConsumerName("consumer").setMaxMemoryUsageBytes(100) - .setRetryConfig(status -> status.getCode() == StatusCode.TRANSPORT_UNAVAILABLE - ? (attempt, elapsed) -> 0 : null) - .withMeter(meter, "reader").build()); - try { - reader.init(); - first.responseInit("first"); - first.responseStartPartition("/topic", 42, 0); - first.responseData(25).partition(1, 0).batch(Codec.RAW, new byte[]{1}).and().send(); - reader.receive(); - first.closeStream(Status.of(StatusCode.TRANSPORT_UNAVAILABLE)); - Assert.assertEquals(1, meter.value(ERRORS)); - meter.assertAttribute(ERRORS, "retry_decision", "retry"); - meter.assertAttribute(ERRORS, "status_code", "TRANSPORT_UNAVAILABLE"); - meter.assertAttribute(ERRORS, "error.type", "transport_error"); - meter.assertNoGauges(); - Assert.assertEquals(1, meter.value(RECEIVED)); - Assert.assertEquals(1, meter.value(DELIVERED)); - Assert.assertEquals(25, meter.value(BYTES)); - - retry.responseInit("retry"); - retry.responseStartPartition("/topic", 42, 1); - Assert.assertEquals(1, meter.collect(PARTITIONS)); - Assert.assertEquals(100, meter.collect(CREDIT)); - retry.responseData(30).partition(1, 1).batch(Codec.RAW, new byte[]{2}).and().send(); - reader.receive(); - Assert.assertEquals(2, meter.value(RECEIVED)); - Assert.assertEquals(2, meter.value(DELIVERED)); - Assert.assertEquals(55, meter.value(BYTES)); - Assert.assertEquals(100, meter.collect(CREDIT)); - - retry.closeStream(Status.of(StatusCode.UNAUTHORIZED)); - Assert.assertEquals(2, meter.value(ERRORS)); - meter.assertAttribute(ERRORS, "retry_decision", "stop"); - meter.assertAttribute(ERRORS, "status_code", "UNAUTHORIZED"); - meter.assertAttribute(ERRORS, "error.type", "ydb_error"); - meter.assertAttribute(ERRORS, "consumer", "consumer"); - meter.assertAttribute(ERRORS, "reader.name", "reader"); - meter.assertNoGauges(); - } finally { - reader.shutdown(); - } + public void gaugesObservePartitionSessionsAndProtocolCredit() throws InterruptedException { + String partitions = "ydb.topic.reader.partition_session.count"; + String credit = "ydb.topic.reader.credit_balance_bytes"; + RecordingMeter meter = new RecordingMeter(); + ReadStreamMock stream = new ReadStreamMock(); + TopicRpc rpc = Mockito.mock(TopicRpc.class); + Mockito.when(rpc.getScheduler()).thenReturn(Mockito.mock(ScheduledExecutorService.class)); + Mockito.when(rpc.readSession(Mockito.anyString())).thenReturn(stream); + TopicClient client = TopicClientImpl.newClient(rpc).build(); + SyncReader reader = client.createSyncReader(ReaderSettings.newBuilder() + .addTopic("/topic").setConsumerName("consumer") + .setMaxMemoryUsageBytes(100).withMeter(meter, "reader").build()); + try { + Assert.assertTrue(meter.gauges.isEmpty()); + reader.init(); + Assert.assertTrue(meter.gauges.isEmpty()); + stream.responseInit("session"); + Assert.assertEquals(0, meter.collect(partitions)); + Assert.assertEquals(100, meter.collect(credit)); + stream.responseStartPartition("/topic", 42, 0); + Assert.assertEquals(1, meter.collect(partitions)); + Assert.assertEquals(100, meter.collect(credit)); + stream.responseData(20).partition(1, 0).batch(Codec.RAW, new byte[]{1}).and().send(); + Assert.assertEquals(80, meter.collect(credit)); + Assert.assertNotNull(reader.receive(1, TimeUnit.SECONDS)); + Assert.assertEquals(100, meter.collect(credit)); + stream.closeStream(Status.of(StatusCode.OVERLOADED)); + Assert.assertTrue(meter.gauges.isEmpty()); + } finally { + reader.shutdown(); + client.close(); } - Assert.assertEquals(2, meter.value(ERRORS)); - meter.assertNoGauges(); + Assert.assertTrue(meter.gauges.isEmpty()); } - private static TopicRpc mockRpc(ReadStreamMock first, ReadStreamMock... retries) { - TopicRpc rpc = Mockito.mock(TopicRpc.class); - Mockito.when(rpc.getScheduler()).thenReturn(Mockito.mock(ScheduledExecutorService.class)); - Mockito.when(rpc.readSession(Mockito.anyString())).thenReturn(first, retries); - return rpc; + private static class RecordingMeter implements Meter { + private final Map counters = new ConcurrentHashMap<>(); + private final Map attributes = new ConcurrentHashMap<>(); + private final Map>> gauges = new ConcurrentHashMap<>(); + + @Override + public MetricRegistration registerLongGauge( + String name, String unit, String description, Consumer callback) { + List> callbacks = gauges.computeIfAbsent(name, key -> new ArrayList<>()); + callbacks.add(callback); + return () -> { + callbacks.remove(callback); + if (callbacks.isEmpty()) { + gauges.remove(name, callbacks); + } + }; + } + + long collect(String name) { + Long[] value = new Long[1]; + Assert.assertEquals(1, gauges.get(name).size()); + gauges.get(name).get(0).accept((observed, attrs) -> value[0] = observed); + return value[0]; + } + + @Override + public LongCounter createCounter(String name, String unit, String description) { + AtomicLong counter = counters.computeIfAbsent(name, key -> new AtomicLong()); + return (value, attrs) -> { + counter.addAndGet(value); + attributes.put(name, attrs); + }; + } + + long value(String name) { + AtomicLong counter = counters.get(name); + return counter == null ? 0 : counter.get(); + } + + void assertAttribute(String name, String key, String value) { + Assert.assertTrue(Arrays.stream(attributes.get(name)) + .anyMatch(attr -> attr.getKey().equals(key) && attr.getValue().equals(value))); + } } } diff --git a/topic/src/test/java/tech/ydb/topic/utils/TopicMeterMock.java b/topic/src/test/java/tech/ydb/topic/utils/TopicMeterMock.java deleted file mode 100644 index 52a62263d..000000000 --- a/topic/src/test/java/tech/ydb/topic/utils/TopicMeterMock.java +++ /dev/null @@ -1,67 +0,0 @@ -package tech.ydb.topic.utils; - -import java.util.Arrays; -import java.util.Map; -import java.util.concurrent.ConcurrentHashMap; -import java.util.concurrent.atomic.AtomicLong; -import java.util.function.Consumer; - -import org.junit.Assert; - -import tech.ydb.core.metrics.Attr; -import tech.ydb.core.metrics.LongCounter; -import tech.ydb.core.metrics.LongMeasurement; -import tech.ydb.core.metrics.Meter; -import tech.ydb.core.metrics.MetricRegistration; - -public class TopicMeterMock implements Meter { - private final Map counters = new ConcurrentHashMap<>(); - private final Map attributes = new ConcurrentHashMap<>(); - private final Map> gauges = new ConcurrentHashMap<>(); - - @Override - public LongCounter createCounter(String name, String unit, String description) { - AtomicLong counter = new AtomicLong(); - Assert.assertNull("Counter registered twice: " + name, counters.putIfAbsent(name, counter)); - return (value, attrs) -> { - attributes.put(name, attrs); - counter.addAndGet(value); - }; - } - - @Override - public MetricRegistration registerLongGauge( - String name, String unit, String description, Consumer callback) { - Assert.assertNull("Gauge registered twice: " + name, gauges.putIfAbsent(name, callback)); - return () -> gauges.remove(name, callback); - } - - public long value(String name) { - AtomicLong counter = counters.get(name); - Assert.assertNotNull("Counter not registered: " + name, counter); - return counter.get(); - } - - public long collect(String name) { - Consumer callback = gauges.get(name); - Assert.assertNotNull("Gauge not registered: " + name, callback); - Long[] value = new Long[1]; - callback.accept((observed, attrs) -> { - attributes.put(name, attrs); - value[0] = observed; - }); - Assert.assertNotNull("Gauge did not record: " + name, value[0]); - return value[0]; - } - - public void assertAttribute(String name, String key, String value) { - Attr[] attrs = attributes.get(name); - Assert.assertNotNull("Metric has no attributes: " + name, attrs); - Assert.assertTrue(name + ": missing " + key + "=" + value, - Arrays.stream(attrs).anyMatch(attr -> attr.getKey().equals(key) && attr.getValue().equals(value))); - } - - public void assertNoGauges() { - Assert.assertTrue("Gauges still registered: " + gauges.keySet(), gauges.isEmpty()); - } -} diff --git a/topic/src/test/java/tech/ydb/topic/write/impl/WriterMetricsTest.java b/topic/src/test/java/tech/ydb/topic/write/impl/WriterMetricsTest.java index 70329498c..1a4e7b03b 100644 --- a/topic/src/test/java/tech/ydb/topic/write/impl/WriterMetricsTest.java +++ b/topic/src/test/java/tech/ydb/topic/write/impl/WriterMetricsTest.java @@ -1,39 +1,44 @@ package tech.ydb.topic.write.impl; -import java.util.concurrent.CompletableFuture; +import java.util.HashMap; +import java.util.Map; import java.util.concurrent.ScheduledExecutorService; +import java.util.function.Consumer; +import java.util.function.LongSupplier; import org.junit.Assert; import org.junit.Test; import org.mockito.Mockito; import tech.ydb.core.Status; -import tech.ydb.core.StatusCode; +import tech.ydb.core.metrics.LongCounter; +import tech.ydb.core.metrics.LongMeasurement; +import tech.ydb.core.metrics.Meter; +import tech.ydb.core.metrics.MetricRegistration; import tech.ydb.topic.TopicClient; import tech.ydb.topic.TopicRpc; import tech.ydb.topic.description.Codec; import tech.ydb.topic.impl.TopicClientImpl; import tech.ydb.topic.settings.WriterSettings; -import tech.ydb.topic.utils.TopicMeterMock; import tech.ydb.topic.write.AsyncWriter; import tech.ydb.topic.write.Message; import tech.ydb.topic.write.QueueOverflowException; -import tech.ydb.topic.write.WriteAck; public class WriterMetricsTest { - private static final String SENDING = "ydb.topic.writer.sending.messages"; - private static final String BYTES = "ydb.topic.writer.sending.bytes"; - private static final String WRITTEN = "ydb.topic.writer.written.messages"; - private static final String ERRORS = "ydb.topic.writer.session.errors"; - private static final String USED = "ydb.topic.writer.buffer.used.bytes"; - private static final String LIMIT = "ydb.topic.writer.buffer.limit.bytes"; @Test - public void writerSendsMessagesAndShutsDown() throws QueueOverflowException { - TopicMeterMock meter = new TopicMeterMock(); + public void writerCountersCountAcceptedMessagesAndAcknowledgements() throws QueueOverflowException { + Map counters = new HashMap<>(); + Meter meter = new Meter() { + @Override + public LongCounter createCounter(String name, String unit, String description) { + counters.put(name, 0L); + return (value, attrs) -> counters.put(name, counters.get(name) + value); + } + }; WriteStreamMock stream = new WriteStreamMock(); WriterSettings settings = WriterSettings.newBuilder() - .setTopicPath("/test/topic").setCodec(Codec.RAW).setMaxSendBufferMemorySize(100) + .setTopicPath("/test/topic").setCodec(Codec.RAW) .withMeter(meter, "writer").build(); try (TopicClient client = TopicClientImpl.newClient(mockRpc(stream)) .setCompressionExecutor(Runnable::run).build()) { @@ -41,106 +46,67 @@ public void writerSendsMessagesAndShutsDown() throws QueueOverflowException { try { writer.init(); stream.sendInitResponse(0); - Assert.assertEquals(0, meter.value(SENDING)); - Assert.assertEquals(0, meter.value(WRITTEN)); - Assert.assertEquals(0, meter.value(ERRORS)); - Assert.assertEquals(0, meter.collect(USED)); - Assert.assertEquals(100, meter.collect(LIMIT)); - - CompletableFuture first = writer.send(Message.of(new byte[]{1, 2})); - CompletableFuture second = writer.send(Message.of(new byte[]{3, 4, 5})); - Assert.assertFalse(first.isDone()); - Assert.assertFalse(second.isDone()); - Assert.assertEquals(2, meter.value(SENDING)); - Assert.assertEquals(5, meter.value(BYTES)); - Assert.assertEquals(0, meter.value(WRITTEN)); - Assert.assertEquals(5, meter.collect(USED)); - Assert.assertEquals(100, meter.collect(LIMIT)); - + writer.send(Message.of(new byte[]{1, 2})); + writer.send(Message.of(new byte[]{3, 4, 5})); stream.sendAckResponse(2, 10); - Assert.assertTrue(first.isDone()); - Assert.assertTrue(second.isDone()); - Assert.assertEquals(2, meter.value(SENDING)); - Assert.assertEquals(5, meter.value(BYTES)); - Assert.assertEquals(2, meter.value(WRITTEN)); - Assert.assertEquals(0, meter.value(ERRORS)); - Assert.assertEquals(0, meter.collect(USED)); - meter.assertAttribute(SENDING, "topic", "/test/topic"); - meter.assertAttribute(BYTES, "writer.name", "writer"); - meter.assertAttribute(WRITTEN, "writer.name", "writer"); - meter.assertAttribute(USED, "topic", "/test/topic"); - meter.assertAttribute(LIMIT, "writer.name", "writer"); + Assert.assertEquals(Long.valueOf(2), counters.get("ydb.topic.writer.sending.messages")); + Assert.assertEquals(Long.valueOf(5), counters.get("ydb.topic.writer.sending.bytes")); + Assert.assertEquals(Long.valueOf(2), counters.get("ydb.topic.writer.written.messages")); } finally { writer.shutdown(); stream.close(Status.SUCCESS); } } - Assert.assertEquals(0, meter.value(ERRORS)); - meter.assertNoGauges(); } @Test - public void writerReconnectsThenStopsOnTerminalError() throws QueueOverflowException { - TopicMeterMock meter = new TopicMeterMock(); - WriteStreamMock first = new WriteStreamMock(); - WriteStreamMock retry = new WriteStreamMock(); + public void writerBufferGaugesTrackBudgetAndLimit() throws QueueOverflowException { + Map gauges = new HashMap<>(); + Meter meter = new Meter() { + @Override + public MetricRegistration registerLongGauge(String name, String unit, String description, + Consumer callback) { + gauges.put(name, () -> { + long[] value = new long[1]; + callback.accept((observed, attrs) -> value[0] = observed); + return value[0]; + }); + return () -> gauges.remove(name); + } + }; + String used = "ydb.topic.writer.buffer.used.bytes"; + String limit = "ydb.topic.writer.buffer.limit.bytes"; + WriteStreamMock stream = new WriteStreamMock(); WriterSettings settings = WriterSettings.newBuilder() .setTopicPath("/test/topic").setCodec(Codec.RAW).setMaxSendBufferMemorySize(100) - .setRetryConfig(status -> status.getCode() == StatusCode.TRANSPORT_UNAVAILABLE - ? (attempt, elapsed) -> 0 : null) .withMeter(meter, "writer").build(); - try (TopicClient client = TopicClientImpl.newClient(mockRpc(first, retry)) + try (TopicClient client = TopicClientImpl.newClient(mockRpc(stream)) .setCompressionExecutor(Runnable::run).build()) { AsyncWriter writer = client.createAsyncWriter(settings); try { writer.init(); - first.sendInitResponse(0); - CompletableFuture written = writer.send(Message.of(new byte[]{1, 2})); - CompletableFuture pending = writer.send(Message.of(new byte[]{3, 4, 5})); - first.close(Status.of(StatusCode.TRANSPORT_UNAVAILABLE)); - Assert.assertEquals(1, meter.value(ERRORS)); - meter.assertAttribute(ERRORS, "retry_decision", "retry"); - meter.assertAttribute(ERRORS, "status_code", "TRANSPORT_UNAVAILABLE"); - meter.assertAttribute(ERRORS, "error.type", "transport_error"); - Assert.assertEquals(2, meter.value(SENDING)); - Assert.assertEquals(5, meter.value(BYTES)); - Assert.assertEquals(0, meter.value(WRITTEN)); - Assert.assertEquals(5, meter.collect(USED)); - Assert.assertEquals(100, meter.collect(LIMIT)); - - retry.sendInitResponse(0); - retry.sendAckResponse(1, 10); - Assert.assertTrue(written.isDone()); - Assert.assertFalse(pending.isDone()); - Assert.assertEquals(2, meter.value(SENDING)); - Assert.assertEquals(5, meter.value(BYTES)); - Assert.assertEquals(1, meter.value(WRITTEN)); - Assert.assertEquals(3, meter.collect(USED)); - - retry.close(Status.of(StatusCode.UNAUTHORIZED)); - Assert.assertTrue(pending.isCompletedExceptionally()); - Assert.assertEquals(2, meter.value(ERRORS)); - Assert.assertEquals(2, meter.value(SENDING)); - Assert.assertEquals(5, meter.value(BYTES)); - Assert.assertEquals(1, meter.value(WRITTEN)); - meter.assertAttribute(ERRORS, "retry_decision", "stop"); - meter.assertAttribute(ERRORS, "status_code", "UNAUTHORIZED"); - meter.assertAttribute(ERRORS, "error.type", "ydb_error"); - meter.assertAttribute(ERRORS, "topic", "/test/topic"); - meter.assertAttribute(ERRORS, "writer.name", "writer"); - meter.assertNoGauges(); + stream.sendInitResponse(0); + Assert.assertEquals(0, gauges.get(used).getAsLong()); + Assert.assertEquals(100, gauges.get(limit).getAsLong()); + writer.send(Message.of(new byte[]{1, 2})); + writer.send(Message.of(new byte[]{3, 4, 5})); + Assert.assertEquals(5, gauges.get(used).getAsLong()); + Assert.assertEquals(100, gauges.get(limit).getAsLong()); + stream.sendAckResponse(2, 10); + Assert.assertEquals(0, gauges.get(used).getAsLong()); } finally { writer.shutdown(); + stream.close(Status.SUCCESS); } } - Assert.assertEquals(2, meter.value(ERRORS)); - meter.assertNoGauges(); + Assert.assertTrue(gauges.isEmpty()); } - private static TopicRpc mockRpc(WriteStreamMock first, WriteStreamMock... retries) { + private static TopicRpc mockRpc(WriteStreamMock stream) { TopicRpc rpc = Mockito.mock(TopicRpc.class); Mockito.when(rpc.getScheduler()).thenReturn(Mockito.mock(ScheduledExecutorService.class)); - Mockito.when(rpc.writeSession(Mockito.anyString())).thenReturn(first, retries); + Mockito.when(rpc.writeSession(Mockito.any(String.class))).thenReturn(stream); return rpc; } + } From f4707c409a949b7dda65b6a6ca65c73e3ac3291c Mon Sep 17 00:00:00 2001 From: KirillKurdyukov Date: Fri, 16 Oct 2026 14:01:05 +0300 Subject: [PATCH 8/9] Distinguish local Reader session failures Classify only explicit local failures as client_error and retain transport classification for RPC errors. Keep the legacy WriterQueue test constructor package-private. --- .../ydb/topic/impl/TopicRetryableStream.java | 34 ++++++++++++------- .../java/tech/ydb/topic/impl/TopicStream.java | 4 +++ .../tech/ydb/topic/impl/TopicStreamBase.java | 26 +++++++++++--- .../tech/ydb/topic/read/impl/ReaderImpl.java | 16 +++++++-- .../ydb/topic/read/impl/ReaderMetrics.java | 4 +-- .../ydb/topic/write/impl/WriterQueue.java | 4 +-- 6 files changed, 64 insertions(+), 24 deletions(-) diff --git a/topic/src/main/java/tech/ydb/topic/impl/TopicRetryableStream.java b/topic/src/main/java/tech/ydb/topic/impl/TopicRetryableStream.java index c8ac9c174..52759693a 100644 --- a/topic/src/main/java/tech/ydb/topic/impl/TopicRetryableStream.java +++ b/topic/src/main/java/tech/ydb/topic/impl/TopicRetryableStream.java @@ -54,12 +54,20 @@ public TopicRetryableStream(Logger logger, String debugId, RetryConfig config, S */ protected abstract void onRetry(@Nullable S stream, Status status); + protected void onRetry(@Nullable S stream, Status status, boolean localFailure) { + onRetry(stream, status); + } + /** * @param stream the closed stream, or {@code null} when stream creation itself failed * @param status status the stream was closed with */ protected abstract void onClose(@Nullable S stream, Status status); + protected void onClose(@Nullable S stream, Status status, boolean localFailure) { + onClose(stream, status); + } + public void start() { if (isClosed) { logger.warn("[{}] double start of closed stream, ignored", debugId); @@ -94,7 +102,7 @@ public void fail(Status status) { State local = state.getAndSet(null); if (local != null) { logger.warn("[{}] failed by application-side error {}", debugId, status); - onStreamStop(local.closeStream(), status, retryConfig.getStatusRetryPolicy(status)); + onStreamStop(local.closeStream(), status, retryConfig.getStatusRetryPolicy(status), true); } } @@ -115,20 +123,20 @@ public boolean close() { return false; } - onStreamStop(local.closeStream(), Status.SUCCESS, null); + onStreamStop(local.closeStream(), Status.SUCCESS, null, false); return true; } - private void onStreamStop(S closed, Status status, RetryPolicy policy) { + private void onStreamStop(S closed, Status status, RetryPolicy policy, boolean localFailure) { if (isClosed) { // stream was already closed (usually with success) - onClose(closed, status); + onClose(closed, status, localFailure); return; } if (policy == null) { logger.warn("[{}] stopped by non-retryable status {}", debugId, status); isClosed = true; - onClose(closed, status); + onClose(closed, status, localFailure); return; } @@ -137,27 +145,27 @@ private void onStreamStop(S closed, Status status, RetryPolicy policy) { if (nextRetryMs < 0) { logger.warn("[{}] stopped after retry policy evaluation for status {}", debugId, status); isClosed = true; - onClose(closed, status); + onClose(closed, status, localFailure); return; } if (nextRetryMs == 0) { // retry immediately logger.warn("[{}] retry #{}. Retry immediately...", debugId, retryState.retryNumber()); - onRetry(closed, status); + onRetry(closed, status, localFailure); start(); return; } // retry scheduling logger.warn("[{}] retry #{}. Scheduling reconnect in {}ms...", debugId, retryState.retryNumber(), nextRetryMs); - onRetry(closed, status); + onRetry(closed, status, localFailure); try { scheduler.schedule(this::start, nextRetryMs, TimeUnit.MILLISECONDS); } catch (Exception ex) { logger.error("[{}] cannot schedule reconnect, stopping", debugId, ex); isClosed = true; - onClose(closed, status); + onClose(closed, status, localFailure); } } @@ -174,14 +182,14 @@ public void start() { if (res == null) { if (state.compareAndSet(this, null)) { Status wrapped = Status.of(StatusCode.CLIENT_INTERNAL_ERROR, th); - onStreamStop(null, wrapped, retryConfig.getThrowableRetryPolicy(th)); + onStreamStop(null, wrapped, retryConfig.getThrowableRetryPolicy(th), false); } return; } if (!res.isSuccess()) { if (state.compareAndSet(this, null)) { - onStreamStop(null, res.getStatus(), retryConfig.getStatusRetryPolicy(res.getStatus())); + onStreamStop(null, res.getStatus(), retryConfig.getStatusRetryPolicy(res.getStatus()), false); } return; } @@ -211,10 +219,10 @@ private void startStream(S local) { } if (status != null) { - onStreamStop(local, status, retryConfig.getStatusRetryPolicy(status)); + onStreamStop(local, status, retryConfig.getStatusRetryPolicy(status), local.isLocalFailure()); } else { Status wrapped = Status.of(StatusCode.CLIENT_INTERNAL_ERROR, th); - onStreamStop(local, wrapped, retryConfig.getThrowableRetryPolicy(th)); + onStreamStop(local, wrapped, retryConfig.getThrowableRetryPolicy(th), false); } }); diff --git a/topic/src/main/java/tech/ydb/topic/impl/TopicStream.java b/topic/src/main/java/tech/ydb/topic/impl/TopicStream.java index 75236073b..d4b6df4b3 100644 --- a/topic/src/main/java/tech/ydb/topic/impl/TopicStream.java +++ b/topic/src/main/java/tech/ydb/topic/impl/TopicStream.java @@ -10,6 +10,10 @@ public interface TopicStream { CompletableFuture start(Consumer messageHandler); + default boolean isLocalFailure() { + return false; + } + void send(W request); void close(); } diff --git a/topic/src/main/java/tech/ydb/topic/impl/TopicStreamBase.java b/topic/src/main/java/tech/ydb/topic/impl/TopicStreamBase.java index 522143003..cd467e8e7 100644 --- a/topic/src/main/java/tech/ydb/topic/impl/TopicStreamBase.java +++ b/topic/src/main/java/tech/ydb/topic/impl/TopicStreamBase.java @@ -16,7 +16,7 @@ public abstract class TopicStreamBase impl private final String debugId; private final GrpcReadWriteStream stream; private final W initRequest; - private final CompletableFuture streamStatus = new CompletableFuture<>(); + private final CompletableFuture streamStatus = new CompletableFuture<>(); private volatile String token; private volatile boolean isStopped = false; @@ -31,6 +31,12 @@ public TopicStreamBase(Logger logger, String debugId, GrpcReadWriteStream protected abstract W updateTokenMessage(String token); protected abstract Status parseMessageStatus(R message); + @Override + public boolean isLocalFailure() { + Completion completion = streamStatus.getNow(null); + return completion != null && completion.localFailure; + } + @Override public CompletableFuture start(Consumer messageHandler) { this.logger.debug("[{}] is about to start", debugId); @@ -40,7 +46,7 @@ public CompletableFuture start(Consumer messageHandler) { messageHandler.accept(msg); } else { logger.warn("[{}] stopped by getting status {}", debugId, messageStatus); - if (streamStatus.complete(messageStatus)) { + if (streamStatus.complete(new Completion(messageStatus, false))) { stream.close(); } } @@ -48,14 +54,14 @@ public CompletableFuture start(Consumer messageHandler) { isStopped = true; Status status = st != null ? st : Status.of(StatusCode.CLIENT_INTERNAL_ERROR, th); logger.debug("[{}] finished with status {}", debugId, status); - streamStatus.complete(status); + streamStatus.complete(new Completion(status, false)); }); if (!streamStatus.isDone()) { stream.sendNext(initRequest); } - return streamStatus; + return streamStatus.thenApply(completion -> completion.status); } @Override @@ -69,7 +75,7 @@ public void close() { protected void fail(Status status) { logger.warn("[{}] stopped by fail {}", debugId, status); - if (streamStatus.complete(status)) { + if (streamStatus.complete(new Completion(status, true))) { isStopped = true; stream.close(); } @@ -92,4 +98,14 @@ public void send(W req) { stream.sendNext(req); } + + private static final class Completion { + private final Status status; + private final boolean localFailure; + + private Completion(Status status, boolean localFailure) { + this.status = status; + this.localFailure = localFailure; + } + } } diff --git a/topic/src/main/java/tech/ydb/topic/read/impl/ReaderImpl.java b/topic/src/main/java/tech/ydb/topic/read/impl/ReaderImpl.java index e643a85e3..630112fc9 100644 --- a/topic/src/main/java/tech/ydb/topic/read/impl/ReaderImpl.java +++ b/topic/src/main/java/tech/ydb/topic/read/impl/ReaderImpl.java @@ -65,9 +65,14 @@ protected CompletableFuture> createNewStream(String id) { protected abstract void onSessionStarted(String sessionId); protected abstract void onReaderClosed(Status status); + @Override + protected void onRetry(ReadSession stream, Status status, boolean localFailure) { + config.getMetrics().reportSessionError(status, true, localFailure); + onRetry(stream, status); + } + @Override protected void onRetry(ReadSession stream, Status status) { - config.getMetrics().reportSessionError(status, true); logger.warn("[{}] stopped by status {}", debugId, status); currentSessionId = null; if (errorHandler != null) { @@ -82,11 +87,18 @@ protected void onRetry(ReadSession stream, Status status) { } } + @Override + protected void onClose(ReadSession stream, Status status, boolean localFailure) { + if (!status.isSuccess()) { + config.getMetrics().reportSessionError(status, false, localFailure); + } + onClose(stream, status); + } + @Override protected void onClose(ReadSession stream, Status status) { currentSessionId = null; if (!status.isSuccess()) { - config.getMetrics().reportSessionError(status, false); logger.warn("[{}] closed by status {}", debugId, status); } else { logger.info("[{}] closed by status {}", debugId, status); diff --git a/topic/src/main/java/tech/ydb/topic/read/impl/ReaderMetrics.java b/topic/src/main/java/tech/ydb/topic/read/impl/ReaderMetrics.java index 3ccd4d520..09c1fa846 100644 --- a/topic/src/main/java/tech/ydb/topic/read/impl/ReaderMetrics.java +++ b/topic/src/main/java/tech/ydb/topic/read/impl/ReaderMetrics.java @@ -73,12 +73,12 @@ void reportReceivedBytes(long bytes) { } } - void reportSessionError(Status status, boolean retry) { + void reportSessionError(Status status, boolean retry, boolean localFailure) { StatusCode code = status.getCode(); Attr[] attributes = Arrays.copyOf(commonAttributes, commonAttributes.length + 3); attributes[commonAttributes.length] = Attr.of("retry_decision", retry ? "retry" : "stop"); attributes[commonAttributes.length + 1] = Attr.of("status_code", code.name()); - String errorType = code == StatusCode.CLIENT_INTERNAL_ERROR ? "client_error" + String errorType = localFailure ? "client_error" : code == StatusCode.SUCCESS || code == StatusCode.UNUSED_STATUS ? "session_closed" : code.isTransportError() ? "transport_error" : "ydb_error"; attributes[commonAttributes.length + 2] = Attr.of("error.type", errorType); diff --git a/topic/src/main/java/tech/ydb/topic/write/impl/WriterQueue.java b/topic/src/main/java/tech/ydb/topic/write/impl/WriterQueue.java index f08a91034..19e5866f8 100644 --- a/topic/src/main/java/tech/ydb/topic/write/impl/WriterQueue.java +++ b/topic/src/main/java/tech/ydb/topic/write/impl/WriterQueue.java @@ -62,8 +62,8 @@ interface EncodedMsg { private volatile CompletableFuture lastAcceptedAckFuture = null; @VisibleForTesting - public WriterQueue(String debugId, WriterSettings settings, CodecRegistry codecRegistry, - Executor compressionExecutor, Runnable readyNotify) { + WriterQueue(String debugId, WriterSettings settings, CodecRegistry codecRegistry, + Executor compressionExecutor, Runnable readyNotify) { this(debugId, settings, new WriterMetrics(Meter.NOOP, settings.getTopicPath(), settings.getWriterName()), codecRegistry, compressionExecutor, readyNotify); } From 73a3c3e67fcb4d03f0241653e4236fbc527c9cde Mon Sep 17 00:00:00 2001 From: KirillKurdyukov Date: Fri, 16 Oct 2026 15:28:55 +0300 Subject: [PATCH 9/9] Restore simple Reader session error reporting --- .../ydb/topic/impl/TopicRetryableStream.java | 34 +++++++------------ .../java/tech/ydb/topic/impl/TopicStream.java | 4 --- .../tech/ydb/topic/impl/TopicStreamBase.java | 26 +++----------- .../tech/ydb/topic/read/impl/ReaderImpl.java | 16 ++------- .../ydb/topic/read/impl/ReaderMetrics.java | 4 +-- 5 files changed, 22 insertions(+), 62 deletions(-) diff --git a/topic/src/main/java/tech/ydb/topic/impl/TopicRetryableStream.java b/topic/src/main/java/tech/ydb/topic/impl/TopicRetryableStream.java index 52759693a..c8ac9c174 100644 --- a/topic/src/main/java/tech/ydb/topic/impl/TopicRetryableStream.java +++ b/topic/src/main/java/tech/ydb/topic/impl/TopicRetryableStream.java @@ -54,20 +54,12 @@ public TopicRetryableStream(Logger logger, String debugId, RetryConfig config, S */ protected abstract void onRetry(@Nullable S stream, Status status); - protected void onRetry(@Nullable S stream, Status status, boolean localFailure) { - onRetry(stream, status); - } - /** * @param stream the closed stream, or {@code null} when stream creation itself failed * @param status status the stream was closed with */ protected abstract void onClose(@Nullable S stream, Status status); - protected void onClose(@Nullable S stream, Status status, boolean localFailure) { - onClose(stream, status); - } - public void start() { if (isClosed) { logger.warn("[{}] double start of closed stream, ignored", debugId); @@ -102,7 +94,7 @@ public void fail(Status status) { State local = state.getAndSet(null); if (local != null) { logger.warn("[{}] failed by application-side error {}", debugId, status); - onStreamStop(local.closeStream(), status, retryConfig.getStatusRetryPolicy(status), true); + onStreamStop(local.closeStream(), status, retryConfig.getStatusRetryPolicy(status)); } } @@ -123,20 +115,20 @@ public boolean close() { return false; } - onStreamStop(local.closeStream(), Status.SUCCESS, null, false); + onStreamStop(local.closeStream(), Status.SUCCESS, null); return true; } - private void onStreamStop(S closed, Status status, RetryPolicy policy, boolean localFailure) { + private void onStreamStop(S closed, Status status, RetryPolicy policy) { if (isClosed) { // stream was already closed (usually with success) - onClose(closed, status, localFailure); + onClose(closed, status); return; } if (policy == null) { logger.warn("[{}] stopped by non-retryable status {}", debugId, status); isClosed = true; - onClose(closed, status, localFailure); + onClose(closed, status); return; } @@ -145,27 +137,27 @@ private void onStreamStop(S closed, Status status, RetryPolicy policy, boolean l if (nextRetryMs < 0) { logger.warn("[{}] stopped after retry policy evaluation for status {}", debugId, status); isClosed = true; - onClose(closed, status, localFailure); + onClose(closed, status); return; } if (nextRetryMs == 0) { // retry immediately logger.warn("[{}] retry #{}. Retry immediately...", debugId, retryState.retryNumber()); - onRetry(closed, status, localFailure); + onRetry(closed, status); start(); return; } // retry scheduling logger.warn("[{}] retry #{}. Scheduling reconnect in {}ms...", debugId, retryState.retryNumber(), nextRetryMs); - onRetry(closed, status, localFailure); + onRetry(closed, status); try { scheduler.schedule(this::start, nextRetryMs, TimeUnit.MILLISECONDS); } catch (Exception ex) { logger.error("[{}] cannot schedule reconnect, stopping", debugId, ex); isClosed = true; - onClose(closed, status, localFailure); + onClose(closed, status); } } @@ -182,14 +174,14 @@ public void start() { if (res == null) { if (state.compareAndSet(this, null)) { Status wrapped = Status.of(StatusCode.CLIENT_INTERNAL_ERROR, th); - onStreamStop(null, wrapped, retryConfig.getThrowableRetryPolicy(th), false); + onStreamStop(null, wrapped, retryConfig.getThrowableRetryPolicy(th)); } return; } if (!res.isSuccess()) { if (state.compareAndSet(this, null)) { - onStreamStop(null, res.getStatus(), retryConfig.getStatusRetryPolicy(res.getStatus()), false); + onStreamStop(null, res.getStatus(), retryConfig.getStatusRetryPolicy(res.getStatus())); } return; } @@ -219,10 +211,10 @@ private void startStream(S local) { } if (status != null) { - onStreamStop(local, status, retryConfig.getStatusRetryPolicy(status), local.isLocalFailure()); + onStreamStop(local, status, retryConfig.getStatusRetryPolicy(status)); } else { Status wrapped = Status.of(StatusCode.CLIENT_INTERNAL_ERROR, th); - onStreamStop(local, wrapped, retryConfig.getThrowableRetryPolicy(th), false); + onStreamStop(local, wrapped, retryConfig.getThrowableRetryPolicy(th)); } }); diff --git a/topic/src/main/java/tech/ydb/topic/impl/TopicStream.java b/topic/src/main/java/tech/ydb/topic/impl/TopicStream.java index d4b6df4b3..75236073b 100644 --- a/topic/src/main/java/tech/ydb/topic/impl/TopicStream.java +++ b/topic/src/main/java/tech/ydb/topic/impl/TopicStream.java @@ -10,10 +10,6 @@ public interface TopicStream { CompletableFuture start(Consumer messageHandler); - default boolean isLocalFailure() { - return false; - } - void send(W request); void close(); } diff --git a/topic/src/main/java/tech/ydb/topic/impl/TopicStreamBase.java b/topic/src/main/java/tech/ydb/topic/impl/TopicStreamBase.java index cd467e8e7..522143003 100644 --- a/topic/src/main/java/tech/ydb/topic/impl/TopicStreamBase.java +++ b/topic/src/main/java/tech/ydb/topic/impl/TopicStreamBase.java @@ -16,7 +16,7 @@ public abstract class TopicStreamBase impl private final String debugId; private final GrpcReadWriteStream stream; private final W initRequest; - private final CompletableFuture streamStatus = new CompletableFuture<>(); + private final CompletableFuture streamStatus = new CompletableFuture<>(); private volatile String token; private volatile boolean isStopped = false; @@ -31,12 +31,6 @@ public TopicStreamBase(Logger logger, String debugId, GrpcReadWriteStream protected abstract W updateTokenMessage(String token); protected abstract Status parseMessageStatus(R message); - @Override - public boolean isLocalFailure() { - Completion completion = streamStatus.getNow(null); - return completion != null && completion.localFailure; - } - @Override public CompletableFuture start(Consumer messageHandler) { this.logger.debug("[{}] is about to start", debugId); @@ -46,7 +40,7 @@ public CompletableFuture start(Consumer messageHandler) { messageHandler.accept(msg); } else { logger.warn("[{}] stopped by getting status {}", debugId, messageStatus); - if (streamStatus.complete(new Completion(messageStatus, false))) { + if (streamStatus.complete(messageStatus)) { stream.close(); } } @@ -54,14 +48,14 @@ public CompletableFuture start(Consumer messageHandler) { isStopped = true; Status status = st != null ? st : Status.of(StatusCode.CLIENT_INTERNAL_ERROR, th); logger.debug("[{}] finished with status {}", debugId, status); - streamStatus.complete(new Completion(status, false)); + streamStatus.complete(status); }); if (!streamStatus.isDone()) { stream.sendNext(initRequest); } - return streamStatus.thenApply(completion -> completion.status); + return streamStatus; } @Override @@ -75,7 +69,7 @@ public void close() { protected void fail(Status status) { logger.warn("[{}] stopped by fail {}", debugId, status); - if (streamStatus.complete(new Completion(status, true))) { + if (streamStatus.complete(status)) { isStopped = true; stream.close(); } @@ -98,14 +92,4 @@ public void send(W req) { stream.sendNext(req); } - - private static final class Completion { - private final Status status; - private final boolean localFailure; - - private Completion(Status status, boolean localFailure) { - this.status = status; - this.localFailure = localFailure; - } - } } diff --git a/topic/src/main/java/tech/ydb/topic/read/impl/ReaderImpl.java b/topic/src/main/java/tech/ydb/topic/read/impl/ReaderImpl.java index 630112fc9..e643a85e3 100644 --- a/topic/src/main/java/tech/ydb/topic/read/impl/ReaderImpl.java +++ b/topic/src/main/java/tech/ydb/topic/read/impl/ReaderImpl.java @@ -65,14 +65,9 @@ protected CompletableFuture> createNewStream(String id) { protected abstract void onSessionStarted(String sessionId); protected abstract void onReaderClosed(Status status); - @Override - protected void onRetry(ReadSession stream, Status status, boolean localFailure) { - config.getMetrics().reportSessionError(status, true, localFailure); - onRetry(stream, status); - } - @Override protected void onRetry(ReadSession stream, Status status) { + config.getMetrics().reportSessionError(status, true); logger.warn("[{}] stopped by status {}", debugId, status); currentSessionId = null; if (errorHandler != null) { @@ -87,18 +82,11 @@ protected void onRetry(ReadSession stream, Status status) { } } - @Override - protected void onClose(ReadSession stream, Status status, boolean localFailure) { - if (!status.isSuccess()) { - config.getMetrics().reportSessionError(status, false, localFailure); - } - onClose(stream, status); - } - @Override protected void onClose(ReadSession stream, Status status) { currentSessionId = null; if (!status.isSuccess()) { + config.getMetrics().reportSessionError(status, false); logger.warn("[{}] closed by status {}", debugId, status); } else { logger.info("[{}] closed by status {}", debugId, status); diff --git a/topic/src/main/java/tech/ydb/topic/read/impl/ReaderMetrics.java b/topic/src/main/java/tech/ydb/topic/read/impl/ReaderMetrics.java index 09c1fa846..3ccd4d520 100644 --- a/topic/src/main/java/tech/ydb/topic/read/impl/ReaderMetrics.java +++ b/topic/src/main/java/tech/ydb/topic/read/impl/ReaderMetrics.java @@ -73,12 +73,12 @@ void reportReceivedBytes(long bytes) { } } - void reportSessionError(Status status, boolean retry, boolean localFailure) { + void reportSessionError(Status status, boolean retry) { StatusCode code = status.getCode(); Attr[] attributes = Arrays.copyOf(commonAttributes, commonAttributes.length + 3); attributes[commonAttributes.length] = Attr.of("retry_decision", retry ? "retry" : "stop"); attributes[commonAttributes.length + 1] = Attr.of("status_code", code.name()); - String errorType = localFailure ? "client_error" + String errorType = code == StatusCode.CLIENT_INTERNAL_ERROR ? "client_error" : code == StatusCode.SUCCESS || code == StatusCode.UNUSED_STATUS ? "session_closed" : code.isTransportError() ? "transport_error" : "ydb_error"; attributes[commonAttributes.length + 2] = Attr.of("error.type", errorType);