yuqi1129 commented on code in PR #13388: URL: https://github.com/apache/gravitino/pull/13388#discussion_r4069633352
########## core/src/main/java/org/apache/gravitino/metrics/source/EntityChangeLogMetricsSource.java: ########## @@ -0,0 +1,116 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +package org.apache.gravitino.metrics.source; + +import com.codahale.metrics.Counter; +import com.codahale.metrics.Gauge; +import com.codahale.metrics.Histogram; +import com.codahale.metrics.Timer; +import java.util.concurrent.atomic.AtomicLong; + +/** Process-local metrics for the entity change log poller and its entity-cache listener. */ +public class EntityChangeLogMetricsSource extends MetricsSource { + private final AtomicLong dbTailId = new AtomicLong(); + private final AtomicLong cursorId = new AtomicLong(); + private final AtomicLong lastSuccessfulPollMs = new AtomicLong(); + private final Counter pollFailures = getCounter("poll-failures-total"); + private final Counter tailSampleFailures = getCounter("tail-sample-failures-total"); + private final Counter listenerFailures = getCounter("listener-failures-total"); + private final Counter recordsFetched = getCounter("records-fetched-total"); + private final Counter recordsDelivered = getCounter("records-delivered-total"); + private final Counter recordsApplied = getCounter("records-applied-total"); + private final Counter invalidationFailures = getCounter("invalidation-failures-total"); + private final Counter fallbackClears = getCounter("fallback-clears-total"); + private final Histogram batchSize = getHistogram("batch-size-records"); + private final Timer pollDuration = getTimer("poll-duration"); + + /** Creates and registers the nonblocking gauges for one server's change log. */ + public EntityChangeLogMetricsSource() { + super("entity-change-log"); + registerGauge("db-tail-id", (Gauge<Long>) dbTailId::get); + registerGauge("cursor-id", (Gauge<Long>) cursorId::get); + registerGauge("record-lag", (Gauge<Long>) () -> Math.max(0, dbTailId.get() - cursorId.get())); + registerGauge( + "seconds-since-last-successful-poll", + (Gauge<Long>) + () -> { + long last = lastSuccessfulPollMs.get(); + return last == 0 ? -1 : Math.max(0, (System.currentTimeMillis() - last) / 1000); + }); + } + + /** Records the database tail sampled by a poll, without querying from the gauge. */ + public void setDbTailId(long id) { + dbTailId.set(id); + } + + /** Records the cursor after a successful delivery. */ + public void setCursorId(long id) { + cursorId.set(id); + } + + /** Records a successful database poll, including an empty result. */ + public void pollSucceeded(int count) { + lastSuccessfulPollMs.set(System.currentTimeMillis()); + recordsFetched.inc(count); + batchSize.update(count); + } + + /** Records a failed poll query or cycle. */ + public void pollFailed() { + pollFailures.inc(); + } + + /** Records a failed database-tail sample while allowing an already fetched batch to proceed. */ + public void tailSampleFailed() { Review Comment: Agreed — the counter alone answers "has it ever failed", which over JMX is the wrong question for the playbook. Added `seconds-since-last-successful-tail-sample` in a5ba0a7496, shaped like `seconds-since-last-successful-poll` (`-1` before the first sample, refreshed in `setDbTailId`), kept the counter for alerting, and rewrote the incident paragraph in `docs/metrics.md` to check the freshness gauge first and read `tail-sample-failures-total` as the alert signal. ########## core/src/test/java/org/apache/gravitino/storage/relational/TestEntityChangeLogPoller.java: ########## @@ -201,12 +207,156 @@ void testPollChangesCatchesFetchFailures() { try (MockedStatic<SessionUtils> sessionUtils = mockStatic(SessionUtils.class)) { mockSessionUtils(sessionUtils, mapper); - EntityChangeLogPoller poller = new EntityChangeLogPoller(1); + EntityChangeLogMetricsSource metrics = new EntityChangeLogMetricsSource(); + EntityChangeLogPoller poller = new EntityChangeLogPoller(1, metrics); Assertions.assertDoesNotThrow(poller::pollChanges); + Assertions.assertEquals( + 1, metrics.getMetricRegistry().counter("poll-failures-total").getCount()); + Assertions.assertEquals( + 0, metrics.getMetricRegistry().counter("records-fetched-total").getCount()); + } + } + + @Test + void testTailSampleFailureDoesNotSuppressFetchedBatch() { + EntityChangeLogMapper mapper = mock(EntityChangeLogMapper.class); + EntityChangeRecord change = change(1L, "CATALOG", "ml1.cat1"); + when(mapper.selectEntityChanges(0L, MAX_ROWS)).thenReturn(List.of(change)); + when(mapper.selectMaxChangeId()).thenThrow(new RuntimeException("tail query failed")); + EntityChangeLogMetricsSource metrics = new EntityChangeLogMetricsSource(); + metrics.setDbTailId(0L); Review Comment: Split into two in a5ba0a7496. `testTailSampleFailureDoesNotSuppressFetchedBatch` drops the no-op seed and now proves the never-sampled state via `seconds-since-last-successful-tail-sample == -1` alongside `db-tail-id == 0` / `record-lag == 0`. New `testTailSampleFailureRetainsPreviousTail` seeds the tail to 9, fails `selectMaxChangeId`, and asserts `db-tail-id` stays 9 with `record-lag` 8. ########## docs/gravitino-server-config.md: ########## @@ -157,22 +160,22 @@ empty string or list; `(none)` means it has no default at all. #### HTTP Server -| Configuration Item | Description | Default Value | -|------------------------------------------------------|----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------|-----------------------------------------| -| `gravitino.server.webserver.host` | The address the server binds to. | `0.0.0.0` | -| `gravitino.server.webserver.httpPort` | The port the server listens on. | `8090` | -| `gravitino.server.webserver.minThreads` | Minimum threads in the Jetty thread pool. Values below 8 are raised to 8. | Twice the processor count, 8 to 100 | -| `gravitino.server.webserver.maxThreads` | Maximum threads in the Jetty thread pool. Values below 8 are raised to 8, and the value must be at least `minThreads`. | Four times the processor count, min 400 | -| `gravitino.server.webserver.threadPoolWorkQueueSize` | Size of the Jetty thread pool work queue. | `100` | -| `gravitino.server.webserver.idleTimeout` | Timeout in milliseconds for idle connections. | `30000` | -| `gravitino.server.webserver.stopTimeout` | Time in milliseconds Jetty waits for a graceful shutdown. See `org.eclipse.jetty.server.Server#setStopTimeout`. | `30000` | -| `gravitino.server.shutdown.timeout` | Time in milliseconds for the Gravitino server itself to shut down gracefully. | `3000` | -| `gravitino.server.webserver.requestHeaderSize` | Maximum size in bytes of an HTTP request header. | `131072` | -| `gravitino.server.webserver.responseHeaderSize` | Maximum size in bytes of an HTTP response header. | `131072` | -| `gravitino.server.webserver.customFilters` | Comma-separated list of servlet filter class names to apply to the API. | (empty) | -| `gravitino.server.rest.extensionPackages` | Comma-separated list of packages to scan for additional REST resources. | (empty) | -| `gravitino.server.visibleConfigs` | Comma-separated list of extra properties to expose on the unauthenticated `GET /configs` endpoint, on top of the fixed set it always returns. Additive, so each entry widens what is public. | (empty) | -| `gravitino.server.bulk.maxItems` | Maximum number of items allowed in a single bulk request. | `100` | +| Configuration Item | Description | Default Value | Review Comment: Reverted in e2d2f235b2; the three files are back to their pre-reformat content. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
