swuferhong commented on code in PR #3388:
URL: https://github.com/apache/fluss/pull/3388#discussion_r3339203355
##########
fluss-server/src/main/java/org/apache/fluss/server/log/LogManager.java:
##########
@@ -386,6 +389,18 @@ private LogTablet loadLog(
PhysicalTablePath physicalTablePath = pathAndBucket.f0;
TablePath tablePath = physicalTablePath.getTablePath();
TableInfo tableInfo = getTableInfo(zkClient, tablePath);
+
+ // Table schema exists, but the partition may have been dropped while
+ // this TS was offline. Check partition registration in ZK.
+ String partitionName = physicalTablePath.getPartitionName();
+ if (partitionName != null && !zkClient.getPartition(tablePath,
partitionName).isPresent()) {
Review Comment:
Validates only partitionName against ZK, not partitionId.
If a partition is dropped and recreated under the same name (new
partitionId), the stale on-disk directory for the old partitionId passes the
check and gets loaded as valid data, while its TableBucket still carries the
old partitionId. Validating partitionId (e.g. comparing the ZK
PartitionRegistration.getPartitionId() with the one parsed from the dir) would
be more robust.
##########
fluss-server/src/main/java/org/apache/fluss/server/log/LogManager.java:
##########
@@ -555,11 +572,23 @@ public void run() {
kvTabletDir);
FileUtils.deleteDirectoryQuietly(kvTabletDir);
}
- } catch (Exception kvDeleteException) {
+
+ boolean isPartitioned =
pathAndBucket.f0.getPartitionName() != null;
+ File parentDir = tabletDir.getParentFile();
Review Comment:
recommend named to `partitionDir`.
##########
fluss-server/src/main/java/org/apache/fluss/server/replica/ReplicaManager.java:
##########
@@ -978,7 +980,20 @@ public void stopReplicas(
TableBucket tb = data.getTableBucket();
HostedReplica hostedReplica = getReplica(tb);
if (hostedReplica instanceof NoneReplica) {
- // do nothing fort this case.
+ if (data.isDeleteLocal()) {
+ try {
+ sweepOrphanTabletDirs(tb, deletedTableIds,
deletedPartitionIds);
Review Comment:
No test to cover this method. Both new tests drop before the TS restart, so
ZK metadata is already absent at startup — this only exercises path A
(SchemaNotExistException handler). The newly added
ReplicaManager.sweepOrphanTabletDirs (path B) has no test coverage.
Consider adding a case that hits the NoneReplica + deleteLocal branch:
restart the TS first (metadata still present → log loaded but no replica), then
drop the table/partition.
--
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]
