Uh oh!
There was an error while loading. Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork 3.4k
HBASE-26328 Clone snapshot doesn't load reference files into FILE SFT impl#3749
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Uh oh!
There was an error while loading. Please reload this page.
Changes from all commits
470bd2e8b26dcc3b9936d17565840ce891a989028b6581fefab03ee1fe91ad4dd7949fcb35f18e6979ce6c712ecae31799150d995c4d7d281727115f02941a27235c12e5a4967d285ffa120c93c8566cf7b03fd2ba9600c9c9789bd1a91bed75b0cecab6140708b7c141ed7e2f2addb15c35744767a3e1be5a148d3556bfc53a80d714e6a0bbaf3e68e3400e1adaf42File filter
Filter by extension
Conversations
Uh oh!
There was an error while loading. Please reload this page.
Jump to
Uh oh!
There was an error while loading. Please reload this page.
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -202,7 +202,6 @@ public static boolean isHFileLink(final Path path) { | ||
| return isHFileLink(path.getName()); | ||
| } | ||
| /** | ||
| * @param fileName File name to check. | ||
| * @return True if the path is a HFileLink. | ||
| @@ -323,10 +322,10 @@ public static String createHFileLinkName(final TableName tableName, | ||
| * @param dstFamilyPath - Destination path (table/region/cf/) | ||
| * @param hfileRegionInfo - Linked HFile Region Info | ||
| * @param hfileName - Linked HFile name | ||
| * @return true if the file is created, otherwise the file exists. | ||
| * @throws IOException on file or parent directory creation failure | ||
| * @return the file link name. | ||
| * @throws IOException on file or parent directory creation failure. | ||
| */ | ||
| public static boolean create(final Configuration conf, final FileSystem fs, | ||
| public static String create(final Configuration conf, final FileSystem fs, | ||
| final Path dstFamilyPath, final RegionInfo hfileRegionInfo, | ||
| final String hfileName) throws IOException { | ||
| return create(conf, fs, dstFamilyPath, hfileRegionInfo, hfileName, true); | ||
| @@ -344,10 +343,10 @@ public static boolean create(final Configuration conf, final FileSystem fs, | ||
| * @param hfileRegionInfo - Linked HFile Region Info | ||
| * @param hfileName - Linked HFile name | ||
| * @param createBackRef - Whether back reference should be created. Defaults to true. | ||
| * @return true if the file is created, otherwise the file exists. | ||
| * @throws IOException on file or parent directory creation failure | ||
| * @return the file link name. | ||
| * @throws IOException on file or parent directory creation failure. | ||
| */ | ||
| public static boolean create(final Configuration conf, final FileSystem fs, | ||
| public static String create(final Configuration conf, final FileSystem fs, | ||
wchevreuil marked this conversation as resolved.
Uh oh!There was an error while loading. Please reload this page. | ||
| final Path dstFamilyPath, final RegionInfo hfileRegionInfo, | ||
| final String hfileName, final boolean createBackRef) throws IOException { | ||
| TableName linkedTable = hfileRegionInfo.getTable(); | ||
| @@ -367,17 +366,18 @@ public static boolean create(final Configuration conf, final FileSystem fs, | ||
| * @param linkedTable - Linked Table Name | ||
| * @param linkedRegion - Linked Region Name | ||
| * @param hfileName - Linked HFile name | ||
| * @return true if the file is created, otherwise the file exists. | ||
| * @throws IOException on file or parent directory creation failure | ||
| * @return the file link name. | ||
| * @throws IOException on file or parent directory creation failure. | ||
| */ | ||
| public static boolean create(final Configuration conf, final FileSystem fs, | ||
| public static String create(final Configuration conf, final FileSystem fs, | ||
wchevreuil marked this conversation as resolved.
Uh oh!There was an error while loading. Please reload this page. | ||
| final Path dstFamilyPath, final TableName linkedTable, final String linkedRegion, | ||
| final String hfileName) throws IOException { | ||
| return create(conf, fs, dstFamilyPath, linkedTable, linkedRegion, hfileName, true); | ||
| } | ||
| /** | ||
| * Create a new HFileLink | ||
| * Create a new HFileLink. In the event of link creation failure, this method throws an | ||
| * IOException, so that the calling upper laying can decide on how to proceed with this. | ||
| * | ||
| * <p>It also adds a back-reference to the hfile back-reference directory | ||
| * to simplify the reference-count and the cleaning process. | ||
| @@ -389,10 +389,10 @@ public static boolean create(final Configuration conf, final FileSystem fs, | ||
| * @param linkedRegion - Linked Region Name | ||
| * @param hfileName - Linked HFile name | ||
| * @param createBackRef - Whether back reference should be created. Defaults to true. | ||
| * @return true if the file is created, otherwise the file exists. | ||
| * @throws IOException on file or parent directory creation failure | ||
| * @return the file link name. | ||
| * @throws IOException on file or parent directory creation failure. | ||
| */ | ||
| public static boolean create(final Configuration conf, final FileSystem fs, | ||
| public static String create(final Configuration conf, final FileSystem fs, | ||
wchevreuil marked this conversation as resolved.
Uh oh!There was an error while loading. Please reload this page. | ||
| final Path dstFamilyPath, final TableName linkedTable, final String linkedRegion, | ||
| final String hfileName, final boolean createBackRef) throws IOException { | ||
| String familyName = dstFamilyPath.getName(); | ||
| @@ -420,7 +420,9 @@ public static boolean create(final Configuration conf, final FileSystem fs, | ||
| } | ||
| try { | ||
| // Create the link | ||
| return fs.createNewFile(new Path(dstFamilyPath, name)); | ||
| if(fs.createNewFile(new Path(dstFamilyPath, name))){ | ||
| return name; | ||
| } | ||
| } catch (IOException e) { | ||
| LOG.error("couldn't create the link=" + name + " for " + dstFamilyPath, e); | ||
| // Revert the reference if the link creation failed | ||
| @@ -429,25 +431,8 @@ public static boolean create(final Configuration conf, final FileSystem fs, | ||
| } | ||
| throw e; | ||
| } | ||
| } | ||
| /** | ||
| * Create a new HFileLink starting from a hfileLink name | ||
| * | ||
| * <p>It also adds a back-reference to the hfile back-reference directory | ||
| * to simplify the reference-count and the cleaning process. | ||
| * | ||
| * @param conf {@link Configuration} to read for the archive directory name | ||
| * @param fs {@link FileSystem} on which to write the HFileLink | ||
| * @param dstFamilyPath - Destination path (table/region/cf/) | ||
| * @param hfileLinkName - HFileLink name (it contains hfile-region-table) | ||
| * @return true if the file is created, otherwise the file exists. | ||
| * @throws IOException on file or parent directory creation failure | ||
| */ | ||
| public static boolean createFromHFileLink(final Configuration conf, final FileSystem fs, | ||
| final Path dstFamilyPath, final String hfileLinkName) | ||
| throws IOException { | ||
| return createFromHFileLink(conf, fs, dstFamilyPath, hfileLinkName, true); | ||
| throw new IOException("File link=" + name + " already exists under " + | ||
| dstFamilyPath + " folder."); | ||
| } | ||
| /** | ||
| @@ -461,10 +446,10 @@ public static boolean createFromHFileLink(final Configuration conf, final FileSy | ||
| * @param dstFamilyPath - Destination path (table/region/cf/) | ||
| * @param hfileLinkName - HFileLink name (it contains hfile-region-table) | ||
| * @param createBackRef - Whether back reference should be created. Defaults to true. | ||
| * @return true if the file is created, otherwise the file exists. | ||
| * @throws IOException on file or parent directory creation failure | ||
| * @return the file link name. | ||
| * @throws IOException on file or parent directory creation failure. | ||
| */ | ||
| public static boolean createFromHFileLink(final Configuration conf, final FileSystem fs, | ||
| public static String createFromHFileLink(final Configuration conf, final FileSystem fs, | ||
wchevreuil marked this conversation as resolved.
Uh oh!There was an error while loading. Please reload this page. | ||
| final Path dstFamilyPath, final String hfileLinkName, final boolean createBackRef) | ||
| throws IOException { | ||
| Matcher m = LINK_NAME_PATTERN.matcher(hfileLinkName); | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -40,7 +40,6 @@ | ||
| import org.apache.hadoop.hbase.master.RegionState; | ||
| import org.apache.hadoop.hbase.master.assignment.AssignmentManager; | ||
| import org.apache.hadoop.hbase.master.procedure.CreateTableProcedure.CreateHdfsRegions; | ||
| import org.apache.hadoop.hbase.mob.MobUtils; | ||
| import org.apache.hadoop.hbase.monitoring.MonitoredTask; | ||
| import org.apache.hadoop.hbase.monitoring.TaskMonitor; | ||
| import org.apache.hadoop.hbase.procedure2.ProcedureStateSerializer; | ||
| @@ -453,56 +452,25 @@ private List<RegionInfo> createFsLayout( | ||
| List<RegionInfo> newRegions, | ||
| final CreateHdfsRegions hdfsRegionHandler) throws IOException { | ||
| final MasterFileSystem mfs = env.getMasterServices().getMasterFileSystem(); | ||
| final Path tempdir = mfs.getTempDir(); | ||
ContributorAuthor There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Snapshots recover is another feature that was relying on temdirs & renames. I took the decision to allow restored dirs being created on the final path already, my understanding is that tables being restored/cloned will not be enabled, and if the snapshot fails at this stage, it will not get to the meta updates stages, meaning there will be no inconsistencies. There would be the need to identify and cleanout leftovers of failed snapshot recoveries. Any thoughts/suggestions? Contributor There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. +1, I think this is similiar to the merge/split scenario. There is no reason that we can not do this. And in #3716 , @frostruan is trying to implement the snapshot operationby proc-v2, so I think we could implement the clean up logic in the rollback of the procedure. | ||
| // 1. Create Table Descriptor | ||
| // using a copy of descriptor, table will be created enabling first | ||
| final Path tempTableDir = CommonFSUtils.getTableDir(tempdir, tableDescriptor.getTableName()); | ||
| if (CommonFSUtils.isExists(mfs.getFileSystem(), tempTableDir)) { | ||
| final Path tableDir = CommonFSUtils.getTableDir(mfs.getRootDir(), | ||
| tableDescriptor.getTableName()); | ||
| if (CommonFSUtils.isExists(mfs.getFileSystem(), tableDir)) { | ||
| // if the region dirs exist, will cause exception and unlimited retry (see HBASE-24546) | ||
| LOG.warn("temp table dir already exists on disk: {}, will be deleted.", tempTableDir); | ||
| CommonFSUtils.deleteDirectory(mfs.getFileSystem(), tempTableDir); | ||
| LOG.warn("temp table dir already exists on disk: {}, will be deleted.", tableDir); | ||
| CommonFSUtils.deleteDirectory(mfs.getFileSystem(), tableDir); | ||
Apache9 marked this conversation as resolved.
Uh oh!There was an error while loading. Please reload this page. | ||
| } | ||
| ((FSTableDescriptors)(env.getMasterServices().getTableDescriptors())) | ||
| .createTableDescriptorForTableDirectory(tempTableDir, | ||
| TableDescriptorBuilder.newBuilder(tableDescriptor).build(), false); | ||
| ((FSTableDescriptors)(env.getMasterServices().getTableDescriptors())) | ||
| .createTableDescriptorForTableDirectory(tableDir, | ||
| TableDescriptorBuilder.newBuilder(tableDescriptor).build(), false); | ||
| // 2. Create Regions | ||
| newRegions = hdfsRegionHandler.createHdfsRegions( | ||
| env, tempdir, tableDescriptor.getTableName(), newRegions); | ||
| // 3. Move Table temp directory to the hbase root location | ||
| CreateTableProcedure.moveTempDirectoryToHBaseRoot(env, tableDescriptor, tempTableDir); | ||
| // Move Table temp mob directory to the hbase root location | ||
| Path tempMobTableDir = MobUtils.getMobTableDir(tempdir, tableDescriptor.getTableName()); | ||
| if (mfs.getFileSystem().exists(tempMobTableDir)) { | ||
| moveTempMobDirectoryToHBaseRoot(mfs, tableDescriptor, tempMobTableDir); | ||
| } | ||
| return newRegions; | ||
| } | ||
| env, mfs.getRootDir(), tableDescriptor.getTableName(), newRegions); | ||
| /** | ||
| * Move table temp mob directory to the hbase root location | ||
| * @param mfs The master file system | ||
| * @param tableDescriptor The table to operate on | ||
| * @param tempMobTableDir The temp mob directory of table | ||
| * @throws IOException If failed to move temp mob dir to hbase root dir | ||
| */ | ||
| private void moveTempMobDirectoryToHBaseRoot(final MasterFileSystem mfs, | ||
| final TableDescriptor tableDescriptor, final Path tempMobTableDir) throws IOException { | ||
| FileSystem fs = mfs.getFileSystem(); | ||
| final Path tableMobDir = | ||
| MobUtils.getMobTableDir(mfs.getRootDir(), tableDescriptor.getTableName()); | ||
| if (!fs.delete(tableMobDir, true) && fs.exists(tableMobDir)) { | ||
| throw new IOException("Couldn't delete mob table " + tableMobDir); | ||
| } | ||
| if (!fs.exists(tableMobDir.getParent())) { | ||
| fs.mkdirs(tableMobDir.getParent()); | ||
| } | ||
| if (!fs.rename(tempMobTableDir, tableMobDir)) { | ||
| throw new IOException("Unable to move mob table from temp=" + tempMobTableDir | ||
| + " to hbase root=" + tableMobDir); | ||
| } | ||
| return newRegions; | ||
| } | ||
| /** | ||
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.