You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@fluss.apache.org by "swuferhong (via GitHub)" <gi...@apache.org> on 2026/10/23 07:55:01 UTC
[PR] [rpc] Reduce Netty shutdown latency and fix lifecycle races [fluss]
swuferhong opened a new pull request, #4817:
URL: https://github.com/apache/fluss/pull/4817
<!--
*Thank you very much for contributing to Fluss - we are happy that you want to help us improve Fluss. To help the community review your contribution in the best possible way, please go through the checklist below, which will get the contribution into a shape in which it can be best reviewed.*
## Contribution Checklist
- Make sure that the pull request corresponds to a [GitHub issue](https://github.com/apache/fluss/issues). Exceptions are made for typos in JavaDoc or documentation files, which need no issue.
- Name the pull request in the format "[component] Title of the pull request", where *[component]* should be replaced by the name of the component being changed. Typically, this corresponds to the component label assigned to the issue (e.g., [kv], [log], [client], [flink], [rust], [python], [c++], [elixir]). Skip *[component]* if you are unsure about which is the best component.
- Fill out the template below to describe the changes contributed by the pull request. That will give reviewers the context they need to do the review.
- Make sure that the change passes the automated tests, i.e., `mvn clean verify` passes.
- Each pull request should address only one issue, not mix up code from multiple issues.
- **Generative AI disclosure:** Indicate whether generative AI tools were used in authoring this PR. If yes, specify the tool below.
- [ ] No generative AI tools used
- [ ] Yes (please specify the tool below)
**(The sections below can be removed for hotfixes or typos)**
-->
<!--
Generated-by: [Tool Name and Version] following [the guidelines](https://github.com/apache/fluss/blob/main/AGENTS.md)
-->
### Purpose
<!-- Linking this pull request to the issue -->
Linked issue: close #4645
<!-- What is the purpose of the change -->
## Summary
This PR reintroduces the Netty shutdown optimization from #4645 together with fixes for the lifecycle and metadata races exposed by faster shutdown.
Removing Netty's default shutdown quiet period changes the timing of cluster teardown, background metadata propagation, and subsequent test setup. Several tests relied on asynchronous work completing during these delays. Other failures exposed production races around table recreation, partition registration, metadata refresh, and tiering shutdown.
The changes are organized into seven commits. Each commit addresses a distinct failure mechanism, with the Netty optimization applied last.
## Changes by commit
### 1. `9292cf587` — Wait for async setup and replica readiness before scans
Several test fixtures started dependent operations before their asynchronous prerequisites had completed, or assumed that table creation implied that all replicas were ready to serve requests.
Changes:
- Wait for database creation before creating tables in `FlinkSinkWriterTest`.
- Preserve concurrent table creation during the rebalance test, but wait for creation to finish before dropping the table during cleanup.
- Remove duplicate asynchronous database teardown from `MultiTableLogScannerITCase`.
- Cancel the Paimon tiering job only once, since its per-job MiniCluster shuts down with the job.
- Create tables before each test in `CoordinatorServiceOrphanRpcsITCase`, matching the cluster extension's per-test cleanup.
- Reuse shared readiness checks in `FlussClusterExtension`. Table and partition waits now check online replicas, table metadata availability, and the local leader state.
- Apply these waits before Flink batch scans, including scans of empty buckets and precreated partitions.
**Failure scenarios:** Missing database/table errors during setup, operations using a table ID removed by the previous test, scans reaching replicas before they become ready, and cleanup requests sent to an already stopped MiniCluster. These failures can depend on test order and cluster startup/teardown timing.
**Coverage:** Adds empty-partition batch-scan coverage for both primary-key and log tables, and updates the affected client, server, Flink, and Paimon fixtures.
### 2. `9e653c44b` — Guard partition registration against concurrent table drops
An auto-partition task can capture a table ID, then attempt partition registration after another thread drops the table or its database. If the same table name is recreated, the task may also target the replacement table using the old ID.
The coordinator's event-processing thread does not serialize every operation involved in this race: auto partitioning and DDL can access the shared metadata manager from different threads.
Changes:
- Use one `ReentrantLock` in `MetadataManager` to serialize partition creation with table deletion and cascading database deletion.
- Validate the captured table ID while holding the lock, and retain the lock through partition registration.
- Perform identity validation before the `ignoreIfExists` early return.
- Treat `TableNotExistException` during auto partitioning as an expected outcome of stale work.
`ZooKeeperClient` and its existing assignment/partition-metadata transaction remain unchanged. The coordination is local to the shared `MetadataManager` instance.
**Failure scenarios:** Delayed partition creation can recreate deleted parent paths, leave orphan partition or assignment metadata, or register metadata for an old table under a reused name. This affects auto-partition tests, concurrent DDL tests, and tests that quickly drop and recreate databases or tables.
**Coverage:** `MetadataManagerTest` and `AutoPartitionManagerTest` cover table/database deletion, same-name recreation, `ignoreIfExists`, and deletion attempting to run after table identity validation but before registration completes.
### 3. `732a9b7f0` — Preserve metadata identity across table recreation
A table path can refer to different table instances over time. Delayed metadata updates or deletion notifications must therefore identify the table being removed, rather than deleting whichever table currently occupies that path.
Changes:
- Carry the original deleted table ID through coordinator metadata notifications using the optional `deleted_table_id` field.
- Remove path mappings only when they still belong to the deleted table or partition instance.
- Ignore older table updates when a newer instance is already cached.
- Clear obsolete bucket assignments, partition mappings, and bucket-count metadata when a table is replaced.
- Avoid combining freshly loaded table information with bucket assignments from an older table instance.
- Preserve legacy deletion handling, including deletion markers reconstructed through RPC decoding.
- Update the Rust protobuf bindings and metadata fixtures for the protocol addition.
**Failure scenarios:** A delayed deletion can remove a newly recreated table from the cache; an older update can restore stale metadata; or a metadata response can combine a new table ID with old bucket assignments. Tests that reuse table or partition names can then encounter missing metadata, incorrect routing, or repeated metadata-refresh failures.
**Coverage:** `CoordinatorRequestBatchTest` and `TabletServerMetadataCacheTest` cover preserved deletion IDs, delayed deletion in the same or a later update batch, table/partition recreation, legacy RPC decoding, and table/bucket identity consistency.
### 4. `de35f9a7c` — Refresh inconsistent metadata through the coordinator
Tablet-server metadata may temporarily lag behind table recreation. A client can receive partitions belonging to an old table instance, or retain old partition mappings after learning that the table name now refers to a new ID.
Changes:
- Detect metadata responses referring to an older or unknown table instance.
- Retry inconsistent table/partition metadata once through the coordinator, including the owning table metadata where available.
- Remove obsolete partition mappings, bucket locations, and bucket counts when a table ID changes.
- Propagate unsuccessful authoritative refreshes instead of treating them as proof that a partition does not exist.
- Preserve existing handling of empty responses during node discovery.
**Failure scenarios:** Clients can report a partition as missing when metadata refresh actually failed, retain stale routing after table recreation, or repeatedly read inconsistent tablet metadata. These scenarios affect client metadata, partitioned read/write, and restart/recovery tests where metadata views temporarily diverge.
**Coverage:** `MetadataUpdaterTest` covers coordinator fallback, inconsistent partition responses, failed coordinator refreshes, empty node-discovery responses, and cleanup of metadata belonging to a replaced table.
### 5. `04e3691ab` — Fail buffered writes with invalid bucket routing
A buffered batch can target a bucket outside the currently known layout. Treating this only as an unknown leader can leave the batch waiting for a leader that will never exist, preventing write callbacks and flush from completing.
However, the cached layout may itself be stale after a bucket expansion, so the client must confirm the layout before rejecting the write.
Changes:
- Distinguish out-of-range buffered buckets from ordinary unknown-leader cases.
- Confirm the layout with the coordinator.
- If the bucket exists in the refreshed layout, continue sending the original buffered records.
- If the routing is confirmed invalid, fail queued and in-flight batches with `InvalidBucketRoutingException` and unblock flush.
- Preserve the original routing of buffered records.
**Failure scenarios:** Write, flush, and writer-cleanup tests can hang or time out after metadata/layout changes. Conversely, rejecting writes based only on a stale tablet cache can fail valid writes during bucket expansion.
**Coverage:** `SenderTest` exercises both outcomes: a valid bucket recovered through coordinator metadata, and confirmed invalid routing that completes queued/in-flight callbacks and releases flush.
### 6. `1f49ab792` — Coordinate tiering shutdown with pending work
The final heartbeat and resource-cleanup logic is already present on `main` and is retained by this PR. This commit adds coordination between shutdown and concurrent tiering work.
Changes:
- Serialize table claims, split publication, reader/event handling, and close where they share lifecycle state.
- Ignore late callbacks and generated results after the enumerator closes.
- Keep potentially slow split generation outside the lifecycle lock, while publishing its result under the lock.
- Ensure the final heartbeat sees the state established by completed table claims.
**Failure scenarios:** Closing or cancelling an enumerator while it claims a table or publishes generated splits can leave work outside the final report. A table may remain assigned to the previous tiering round until the coordinator timeout, causing tiering restart, cancellation, and recovery tests to stall while waiting to reclaim it.
**Coverage:** `TieringSourceEnumeratorTest` verifies that empty, completed, and unfinished rounds can be reclaimed after close without waiting for the next periodic heartbeat. Existing scheduling and cleanup tests also validate the combined behavior with `main`.
### 7. `c17c68b96` — Reduce Netty shutdown latency
Netty's default graceful shutdown includes a two-second quiet period. Removing that delay requires explicit ordering between connection shutdown, request draining, and event-loop termination.
Changes:
- Shut down event-loop groups with a zero quiet period.
- Close client connections before terminating their event loop, while still terminating it if connection cleanup fails.
- Close connections created concurrently with or after client shutdown so their requests complete exceptionally.
- Keep server event loops alive until listening channels are closed and accepted requests are drained.
- Tolerate rejected backpressure tasks when their event loop is shutting down.
**Failure scenarios:** Premature event-loop termination can prevent accepted requests from completing, leave newly created connections with unresolved requests, or let a rejected backpressure task terminate a request-processing thread. These issues can appear as shutdown hangs, unresolved futures, or timeouts in RPC lifecycle and cluster restart/failover tests.
**Coverage:** `NettyClientTest`, `NettyServerTest`, and `RequestChannelTest` cover connections created after close, responses to accepted requests during shutdown, and rejected event-loop submissions.
## Validation
After rebasing onto `main`, 168 targeted tests passed with no failures, errors, or skipped tests:
| Area | Tests executed | Count |
| --- | --- | ---: |
| RPC | `NettyClientTest`, `NettyServerTest`, `NettyServerTlsTest`, `RequestChannelTest` | 20 |
| Partition lifecycle | `MetadataManagerTest`, `AutoPartitionManagerTest` | 32 |
| Client | `SenderTest`, `MetadataUpdaterTest`, `FlussAdminITCase#testKvSnapshotLeaseAfterCoordinatorServerRestart` | 93 |
| Flink tiering | `TieringSourceEnumeratorTest`, `TieringSourceEnumeratorSchedulingTest` | 23 |
Checkstyle, Spotless, and license checks passed for this build.
The regression coverage described above includes tests added or updated by the commits. The table lists the targeted tests executed after the rebase; the full CI matrix was not run locally.
### Brief change log
<!-- Please describe the changes made in this pull request and explain how they address the issue -->
### Tests
<!-- List UT and IT cases to verify this change -->
### API and Format
<!-- Does this change affect API or storage format -->
### Documentation
<!-- Does this change introduce a new feature -->
--
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: issues-unsubscribe@fluss.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [rpc] Reduce Netty shutdown latency and fix lifecycle races [fluss]
Posted by "polyzos (via GitHub)" <gi...@apache.org>.
polyzos commented on code in PR #4817:
URL: https://github.com/apache/fluss/pull/4817#discussion_r4389644974
##########
fluss-client/src/main/java/org/apache/fluss/client/write/Sender.java:
##########
@@ -255,13 +264,41 @@ private void sendWriteData() throws Exception {
// if there are any buckets whose leaders are not known yet, force metadata update
if (!readyCheckResult.unknownLeaderTables.isEmpty()) {
try {
+ if (!readyCheckResult.invalidBucketTables.isEmpty()) {
+ // A cached layout can lag a bucket expansion. Confirm with the coordinator
+ // before failing a batch that cannot reach a server for routing validation.
+ Cluster refreshedCluster =
Review Comment:
This blocking coordinator call runs on the sender thread inside a synchronized method, and its time budget is a hard-coded 30s. If the coordinator is slow, every table sharing the writer drains about once every 30s. Could this run asynchronously, or maybe use client.request-timeout?
##########
fluss-client/src/main/java/org/apache/fluss/client/write/RecordAccumulator.java:
##########
@@ -620,14 +626,46 @@ void rerouteQueuedWritesToHistorical(
/** Aborts incomplete batches whose current RPC target is {@code targetPath}. */
void abortBatches(PhysicalTablePath targetPath, Exception reason) {
+ if (isTargetFailed(targetPath)) {
+ // An append may have passed the failure check before the target was fenced. Wait
+ // for its batch registration before taking the incomplete-batch snapshot.
+ for (BucketAndWriteBatches context : writeBatches.values()) {
+ synchronized (context) {
+ if (context.targetPath.equals(targetPath)) {
+ for (Deque<WriteBatch> deque : context.batches.values()) {
+ synchronized (deque) {
+ // Wait for an append already holding this deque.
+ }
+ }
+ }
+ }
+ }
+ }
for (WriteBatch batch : incomplete.copyAll()) {
BucketAndWriteBatches writeTarget = writeBatches.get(batch.physicalTablePath());
if (writeTarget != null && writeTarget.targetPath.equals(targetPath)) {
abortBatch(reason, batch);
+ if (idempotenceManager.idempotenceEnabled() && batch.hasBatchSequence()) {
+ idempotenceManager.removeInFlightBatch(
+ new ReadyWriteBatch(
+ new TableBucket(
+ batch.tableId(),
+ writeTarget.partitionId,
+ batch.bucketId()),
+ batch));
+ }
}
}
}
+ void failTarget(PhysicalTablePath targetPath, Exception reason) {
Review Comment:
failedTargets is never cleared, and it's keyed by path rather than table id. So once a table is recreated with fewer buckets, writes to the new table keep failing for as long as the connection stays open. Could the key include the table id, or could the entry be cleared once the old batches are aborted?
##########
fluss-server/src/main/java/org/apache/fluss/server/coordinator/MetadataManager.java:
##########
@@ -101,6 +104,10 @@ public class MetadataManager {
private final int maxBucketNum;
private final LakeCatalogDynamicLoader lakeCatalogDynamicLoader;
+ // Auto partitioning and DDL share this manager. Keep table identity validation and
+ // partition registration atomic with table deletion, including cascading database drops.
+ private final Lock partitionRegistrationLock = new ReentrantLock();
Review Comment:
This single lock is held across several ZooKeeper round trips, so all partition creations across all tables are serialized and wait behind database drops. It also only protects within one process. Could a check on the table node inside the ZooKeeper transaction give the same guarantee without the global lock?
##########
fluss-client/src/main/java/org/apache/fluss/client/write/Sender.java:
##########
@@ -255,13 +264,41 @@ private void sendWriteData() throws Exception {
// if there are any buckets whose leaders are not known yet, force metadata update
if (!readyCheckResult.unknownLeaderTables.isEmpty()) {
try {
+ if (!readyCheckResult.invalidBucketTables.isEmpty()) {
+ // A cached layout can lag a bucket expansion. Confirm with the coordinator
+ // before failing a batch that cannot reach a server for routing validation.
+ Cluster refreshedCluster =
+ metadataUpdater.updatePhysicalTableMetadataFromCoordinator(
+ readyCheckResult.invalidBucketTables);
+ Set<PhysicalTablePath> invalidTables =
+ new HashSet<>(accumulator.ready(refreshedCluster).invalidBucketTables);
+ invalidTables.retainAll(readyCheckResult.invalidBucketTables);
+ if (!invalidTables.isEmpty()) {
+ InvalidBucketRoutingException failure =
+ new InvalidBucketRoutingException(
+ "Buffered records target buckets outside the current layout for "
+ + invalidTables
+ + ". Recreate the connection with current metadata.");
+ invalidTables.forEach(path -> failRoutingTarget(path, failure));
+ abortInvalidRoutingTargets();
+ }
+ return;
+ }
metadataUpdater.updatePhysicalTableMetadata(readyCheckResult.unknownLeaderTables);
} catch (Exception e) {
// TODO: this try-catch is not needed when we don't update metadata for
// unready partitions
Throwable t = ExceptionUtils.stripExecutionException(e);
if (t instanceof PartitionNotExistException) {
handlePartitionNotExistException(readyCheckResult.unknownLeaderTables);
+ } else if (!readyCheckResult.invalidBucketTables.isEmpty()
+ && (t instanceof RetriableException
+ || t instanceof NotCoordinatorLeaderException
+ || t instanceof TimeoutException)) {
+ // Leave the unresolved target queued, but let healthy targets below drain.
+ // A fast rejection from the previous leader must not cause a tight retry loop.
+ LOG.debug("Routing metadata is temporarily unavailable; will retry.", t);
+ awaitNextReadyCheck(100);
} else {
Review Comment:
he regular metadata refresh can now throw StaleMetadataException, where on main it only logged the error. When no table has an invalid bucket, the exception reaches this throw and the sender loop retries straight away. So during a coordinator failover, all tables stop draining and the log fills up. Maybe handle this like the retriable case above, with a backoff?
##########
fluss-rpc/src/main/java/org/apache/fluss/rpc/netty/server/NettyServer.java:
##########
@@ -304,24 +304,28 @@ public CompletableFuture<Void> closeAsync() {
isRunning = false;
- CompletableFuture<Void> acceptorShutdownFuture = shutdownGroup(acceptorGroup);
- CompletableFuture<Void> selectorShutdownFuture = shutdownGroup(selectorGroup);
+ // Stop accepting new connections.
CompletableFuture<Void> channelShutdownFuture =
FutureUtils.completeAll(
bindChannels.stream()
.map(NettyUtils::shutdownChannel)
.collect(Collectors.toList()));
+ // Drain the requests that were already accepted.
CompletableFuture<Void> workerShutdownFuture;
if (workerPool != null) {
workerShutdownFuture = workerPool.closeAsync();
} else {
workerShutdownFuture = CompletableFuture.completedFuture(null);
}
-
- return CompletableFuture.allOf(
- acceptorShutdownFuture,
- selectorShutdownFuture,
- channelShutdownFuture,
- workerShutdownFuture);
+ // Shut down the event loops only after both of the above completed. The event loops
+ // terminate without a quiet period, so they must outlive the tasks still submitted to
+ // them: the bind channel close listeners, and the responses (and backpressure updates)
+ // of the requests drained by the worker pool. Shutting the event loops down closes the
+ // remaining client connections.
+ return FutureUtils.composeAfterwards(
Review Comment:
Connections now stay open during the drain. Requests that arrive after the shutdown marker are never processed or answered, and their buffers are never released. The drain also has no time limit, so a stuck handler would keep client connections open indefinitely. Could we reject requests that arrive once closing has started, and add a time limit to the drain?
##########
fluss-flink/fluss-flink-common/src/main/java/org/apache/fluss/flink/tiering/source/enumerator/TieringSourceEnumerator.java:
##########
@@ -625,14 +670,28 @@ public TieringSourceEnumeratorState snapshotState(long checkpointId) throws Exce
@Override
public void close() throws IOException {
- // Repeated lifecycle calls are a no-op: the final report is attempted once and the
- // resources are released once.
- if (closed) {
- return;
+ final CompletableFuture<Void> pendingHeartbeat;
+ synchronized (this) {
+ if (closed) {
+ return;
+ }
+ closed = true;
+ pendingHeartbeat = heartbeatInFlight;
}
- closed = true;
timerService.shutdownNow();
try {
+ if (pendingHeartbeat != null) {
+ // The heartbeat itself has a bounded RPC wait. Wait outside the monitor so
+ // reader events can observe closed without waiting for that RPC.
+ try {
Review Comment:
This wait can last up to 3 minutes, but Flink interrupts a coordinator that takes more than 60s to close. In that case close() exits before sending the final report, and the in-flight tables stay stuck until the coordinator timeout. Maybe wait for less time here, and still send the final report if the wait fails?
--
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: issues-unsubscribe@fluss.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [rpc] Reduce Netty shutdown latency and fix lifecycle races [fluss]
Posted by "polyzos (via GitHub)" <gi...@apache.org>.
polyzos commented on PR #4817:
URL: https://github.com/apache/fluss/pull/4817#issuecomment-6444233806
Thanks @swuferhong for handling all The earlier comments are all addressed, but i left a few more, if you think something is better on a follow-up im fine with it and after a quick pass on the above I think we can merge.
Thanks again for tracking all of these down!
--
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: issues-unsubscribe@fluss.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [rpc] Reduce Netty shutdown latency and fix lifecycle races [fluss]
Posted by "polyzos (via GitHub)" <gi...@apache.org>.
polyzos commented on code in PR #4817:
URL: https://github.com/apache/fluss/pull/4817#discussion_r4357662485
##########
fluss-server/src/main/java/org/apache/fluss/server/coordinator/CoordinatorRequestBatch.java:
##########
Review Comment:
Tables now keep their real id when they're being deleted, but partitions are still matched by name. A delayed delete could still remove a new partition with the same name. Could partitions get the same treatment?
##########
fluss-client/src/main/java/org/apache/fluss/client/write/Sender.java:
##########
@@ -255,6 +256,26 @@ private void sendWriteData() throws Exception {
// if there are any buckets whose leaders are not known yet, force metadata update
if (!readyCheckResult.unknownLeaderTables.isEmpty()) {
try {
+ if (!readyCheckResult.invalidBucketTables.isEmpty()) {
+ // A cached layout can lag a bucket expansion. Confirm with the coordinator
+ // before failing a batch that cannot reach a server for routing validation.
+ Cluster refreshedCluster =
+ metadataUpdater.updatePhysicalTableMetadataFromCoordinator(
+ readyCheckResult.invalidBucketTables);
+ Set<PhysicalTablePath> invalidTables =
+ new HashSet<>(accumulator.ready(refreshedCluster).invalidBucketTables);
+ invalidTables.retainAll(readyCheckResult.invalidBucketTables);
+ if (!invalidTables.isEmpty()) {
+ InvalidBucketRoutingException failure =
+ new InvalidBucketRoutingException(
+ "Buffered records target buckets outside the current layout for "
+ + invalidTables
+ + ". Recreate the writer with current metadata.");
+ recordFatalError(failure);
Review Comment:
This fails the whole writer and drops every buffered batch. The writer is shared per connection, so one recreated table would fail writes to all other tables. Should we fail only the batches for the affected tables?
##########
fluss-client/src/main/java/org/apache/fluss/client/metadata/MetadataUpdater.java:
##########
@@ -275,7 +295,18 @@ public void updateMetadata(
}
} catch (Exception e) {
Throwable t = stripExecutionException(e);
- if (t instanceof RetriableException || t instanceof TimeoutException) {
+ if (refreshingTableMetadata && t instanceof StaleMetadataException) {
Review Comment:
This new error is retriable, but the dynamic partition creation path wraps anything other than "partition doesn't exist" into a non-retriable error. So writing to a table while it's being recreated now fails instead of retrying. Should we handle this error there, or keep the previous behaviour for that path?
##########
fluss-flink/fluss-flink-common/src/main/java/org/apache/fluss/flink/tiering/source/enumerator/TieringSourceEnumerator.java:
##########
@@ -445,7 +454,9 @@ private void assignSplits() {
}
}
- private @Nullable Tuple3<Long, Long, TablePath> requestTieringTableSplitsViaHeartBeat() {
+ // Serialize claims with close so its final heartbeat includes every claimed table.
+ private synchronized @Nullable Tuple3<Long, Long, TablePath>
Review Comment:
This holds the lock while waiting for the heartbeat reply, which can take up to 3 minutes. Adding readers, handling finished-table events and assigning splits all run on Flink's coordinator thread and wait on the same lock, so a slow coordinator would stall the enumerator. Should the lock cover only the state updates, with the claimed table recorded once the reply arrives? A test with the heartbeat and a source event running at the same time would also help.
##########
fluss-rpc/src/main/proto/FlussApi.proto:
##########
@@ -1001,8 +1001,9 @@ message PbTableMetadata {
// A table-level, monotonically increasing version for bucket.num changes.
optional int64 bucket_count_epoch = 9;
- // TODO add a new filed 'deleted_table' to indicate this table is deleted in UpdateMetadataRequest.
- // trace by: https://github.com/apache/fluss/issues/981
+ // Original ID of a deleted table. Keep the legacy table_id/path tombstone for older servers.
+ // The ID prevents a delayed deletion from removing a newly created table with the same name.
+ optional int64 deleted_table_id = 10;
Review Comment:
Is a new field needed here? Sending the deleted-table path together with the real table id already carries the same information. Older tablet servers already handle that combination safely, so we could avoid the protocol change.
--
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: issues-unsubscribe@fluss.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [rpc] Reduce Netty shutdown latency and fix lifecycle races [fluss]
Posted by "polyzos (via GitHub)" <gi...@apache.org>.
polyzos commented on PR #4817:
URL: https://github.com/apache/fluss/pull/4817#issuecomment-6361105469
@swuferhong Thanks for tracking these down 🙏 I left a few comments
--
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: issues-unsubscribe@fluss.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [rpc] Reduce Netty shutdown latency and fix lifecycle races [fluss]
Posted by "swuferhong (via GitHub)" <gi...@apache.org>.
swuferhong commented on PR #4817:
URL: https://github.com/apache/fluss/pull/4817#issuecomment-6406110213
> @swuferhong Thanks for tracking these down 🙏 I left a few comments
Hi, @polyzos These issues have been fixed, but I folded the review fixes into the existing commits in this PR rather than keeping them in a separate commit. Could you please take another look? I’ll keep fixes for any further review comments in separate commits.
--
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: issues-unsubscribe@fluss.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org