Copilot commented on code in PR #8233:
URL: https://github.com/apache/hbase/pull/8233#discussion_r3367534997
##########
hbase-server/src/test/java/org/apache/hadoop/hbase/master/cleaner/TestHFileLinkCleaner.java:
##########
@@ -24,6 +24,7 @@
import java.io.IOException;
import java.util.Collections;
import org.apache.hadoop.conf.Configuration;
+import org.apache.hadoop.hbase.regionserver.StoreFileInfo;
import org.apache.hadoop.fs.FileStatus;
import org.apache.hadoop.fs.FileSystem;
import org.apache.hadoop.fs.Path;
Review Comment:
The newly added StoreFileInfo import is duplicated later in the file and
also breaks import ordering (it is placed between org.apache.hadoop.conf.* and
org.apache.hadoop.fs.*). Remove this redundant import and keep the existing one
in the regionserver import block.
##########
hbase-server/src/test/java/org/apache/hadoop/hbase/master/cleaner/TestHFileLinkCleaner.java:
##########
@@ -211,6 +212,56 @@ public void testHFileLinkEmptyBackReferenceDirectory()
throws Exception {
assertFalse(fs.exists(linkBackRefDir), "back reference directory should be
deleted");
}
+ /**
+ * Verify that an HFileLink Reference file — created by
+ * {@code RestoreSnapshotHelper.restoreReferenceFile()} when cloning a
snapshot that contains
+ * split/merge reference files — protects the archived HFile from deletion by
+ * {@link HFileLinkCleaner}.
+ *
+ * <p>Unlike a normal clone (which calls {@code HFileLink.create()} and gets
a zero-byte HFileLink
+ * file plus a back-reference), {@code restoreReferenceFile()} writes only a
Reference file whose
+ * base name is an HFileLink name (e.g. {@code
srcTable=srcRegion-hfile.cloneRegion}). The
+ * back-reference points to a path that {@code HFileLinkCleaner}
reconstructs as a zero-byte
+ * HFileLink file — but that file does not exist. The cleaner must also look
for Reference files
+ * at that path before concluding the forward link is gone.
+ */
+ @Test
+ public void testHFileLinkReferenceFileProtectsArchivedHFile() throws
Exception {
+ // @Before created: archived HFile, zero-byte HFileLink, and
back-reference.
+ // restoreReferenceFile() does NOT create a zero-byte HFileLink file; it
creates a Reference
+ // file whose base name is the HFileLink name. Simulate that here.
+
+ // Step 1: remove the zero-byte HFileLink (restoreReferenceFile doesn't
create one).
+ assertTrue(fs.delete(new Path(familyLinkPath, hfileLinkName), false));
+
+ // Step 2: create the HFileLink Reference file that restoreReferenceFile()
produces.
+ // Name pattern: <table>=<srcRegion>-<hfile>.<cloneRegionEncoded>
+ String hfileLinkRefName = hfileLinkName + "." + hriLink.getEncodedName();
+ Path hfileLinkRefPath = new Path(familyLinkPath, hfileLinkRefName);
+ fs.createNewFile(hfileLinkRefPath);
+ // Sanity-check: the naming must satisfy StoreFileInfo.isReference() so
HBase treats it as a
+ // reference file, not a plain HFile.
+ assertTrue(StoreFileInfo.isReference(hfileLinkRefPath),
+ "HFileLink Reference name must be recognized as a reference file");
+
+ // The back-reference dir+file already exists from @Before.
+ // The cleaner must NOT delete the archived HFile while the Reference file
still exists.
+ cleaner.chore();
+ assertTrue(fs.exists(hfilePath),
+ "Archived HFile must be protected while an HFileLink Reference file
exists in the clone");
+
+ // Step 3: simulate clone-table compaction — the Reference file is
replaced by a real HFile
+ // and removed from the store.
+ assertTrue(fs.delete(hfileLinkRefPath, false));
+
+ // The back-reference's forward link is now gone; the cleaner should mark
the back-ref
+ // deletable and remove it.
+ cleaner.chore();
+ assertFalse(fs.exists(linkBackRef),
+ "Back-reference should be removed after HFileLink Reference file is gone");
+ // Archived HFile cleanup (requires TTL to expire) is validated by @After.
Review Comment:
Indentation for the wrapped assertFalse call does not match the configured
continuation indentation (line-wrapping indent is 2 spaces). This will fail
Checkstyle Indentation.
##########
hbase-server/src/test/java/org/apache/hadoop/hbase/client/CloneSnapshotFromClientAfterMergingRegionTestBase.java:
##########
@@ -0,0 +1,320 @@
+/*
+ * 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.hadoop.hbase.client;
+
+import static org.junit.jupiter.api.Assertions.assertTrue;
+
+import java.io.IOException;
+import java.util.ArrayList;
+import java.util.List;
+import org.apache.hadoop.fs.FileStatus;
+import org.apache.hadoop.fs.FileSystem;
+import org.apache.hadoop.fs.Path;
+import org.apache.hadoop.hbase.TableName;
+import org.apache.hadoop.hbase.master.RegionState;
+import org.apache.hadoop.hbase.master.assignment.RegionStates;
+import org.apache.hadoop.hbase.master.cleaner.HFileCleaner;
+import org.apache.hadoop.hbase.master.cleaner.TimeToLiveHFileCleaner;
+import org.apache.hadoop.hbase.master.snapshot.SnapshotHFileCleaner;
+import org.apache.hadoop.hbase.regionserver.CompactedHFilesDischarger;
+import org.apache.hadoop.hbase.regionserver.HRegion;
+import org.apache.hadoop.hbase.io.HFileLink;
+import org.apache.hadoop.hbase.regionserver.HStore;
+import org.apache.hadoop.hbase.regionserver.HStoreFile;
+import org.apache.hadoop.hbase.regionserver.StoreFileInfo;
Review Comment:
Import ordering: org.apache.hadoop.hbase.io.HFileLink should appear before
org.apache.hadoop.hbase.master.* and org.apache.hadoop.hbase.regionserver.*
imports to satisfy Checkstyle ImportOrder (lexicographic ordering within the
top-level import group).
##########
hbase-server/src/main/java/org/apache/hadoop/hbase/snapshot/RestoreSnapshotHelper.java:
##########
@@ -42,6 +42,7 @@
import org.apache.hadoop.hbase.backup.HFileArchiver;
import org.apache.hadoop.hbase.client.ColumnFamilyDescriptor;
import org.apache.hadoop.hbase.client.ColumnFamilyDescriptorBuilder;
+import org.apache.hadoop.hbase.util.HFileArchiveUtil;
import org.apache.hadoop.hbase.client.Connection;
import org.apache.hadoop.hbase.client.ConnectionFactory;
Review Comment:
New import for HFileArchiveUtil breaks the repository's Checkstyle import
ordering (imports are required to be lexicographically ordered within the
top-level group). It should be moved down with the other
org.apache.hadoop.hbase.util.* imports to keep the block sorted.
--
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]
