-
Notifications
You must be signed in to change notification settings - Fork 3.3k
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
HBASE-26328 Clone snapshot doesn't load reference files into FILE SFT impl #3749
Changes from 27 commits
470bd2e
8b26dcc
3b9936d
1756584
0ce891a
989028b
6581fef
ab03ee1
fe91ad4
dd7949f
cb35f18
e6979ce
6c712ec
ae31799
150d995
c4d7d28
1727115
f02941a
27235c1
2e5a496
7d285ff
a120c93
c8566cf
7b03fd2
ba9600c
9c9789b
d1a91be
d75b0ce
cab6140
708b7c1
41ed7e2
f2addb1
5c35744
767a3e1
be5a148
d3556bf
c53a80d
714e6a0
bbaf3e6
8e3400e
1adaf42
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,36 @@ | ||
/** | ||
* 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. | ||
*/ | ||
syntax = "proto2"; | ||
// This file contains protocol buffers that are used for store file tracker. | ||
package hbase.pb; | ||
|
||
option java_package = "org.apache.hadoop.hbase.shaded.protobuf.generated"; | ||
option java_outer_classname = "StoreFileTrackerProtos"; | ||
option java_generic_services = true; | ||
option java_generate_equals_and_hash = true; | ||
option optimize_for = SPEED; | ||
|
||
message StoreFileEntry { | ||
required string name = 1; | ||
required uint64 size = 2; | ||
} | ||
|
||
message StoreFileList { | ||
required uint64 timestamp = 1; | ||
repeated StoreFileEntry store_file = 2; | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -29,6 +29,7 @@ | |
import org.apache.hadoop.hbase.client.RegionInfo; | ||
import org.apache.hadoop.hbase.client.RegionInfoBuilder; | ||
import org.apache.hadoop.hbase.mob.MobConstants; | ||
import org.apache.hadoop.hbase.mob.MobUtils; | ||
import org.apache.hadoop.hbase.regionserver.HRegion; | ||
import org.apache.hadoop.hbase.regionserver.StoreFileInfo; | ||
import org.apache.hadoop.hbase.util.CommonFSUtils; | ||
|
@@ -202,7 +203,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. | ||
|
@@ -326,7 +326,7 @@ public static String createHFileLinkName(final TableName tableName, | |
* @return true if the file is created, otherwise the file exists. | ||
* @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.
Show resolved
Hide resolved
|
||
final Path dstFamilyPath, final RegionInfo hfileRegionInfo, | ||
final String hfileName) throws IOException { | ||
return create(conf, fs, dstFamilyPath, hfileRegionInfo, hfileName, true); | ||
|
@@ -347,7 +347,7 @@ public static boolean create(final Configuration conf, final FileSystem fs, | |
* @return true if the file is created, otherwise the file exists. | ||
* @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.
Show resolved
Hide resolved
|
||
final Path dstFamilyPath, final RegionInfo hfileRegionInfo, | ||
final String hfileName, final boolean createBackRef) throws IOException { | ||
TableName linkedTable = hfileRegionInfo.getTable(); | ||
|
@@ -370,7 +370,7 @@ public static boolean create(final Configuration conf, final FileSystem fs, | |
* @return true if the file is created, otherwise the file exists. | ||
* @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.
Show resolved
Hide resolved
|
||
final Path dstFamilyPath, final TableName linkedTable, final String linkedRegion, | ||
final String hfileName) throws IOException { | ||
return create(conf, fs, dstFamilyPath, linkedTable, linkedRegion, hfileName, true); | ||
|
@@ -392,7 +392,7 @@ public static boolean create(final Configuration conf, final FileSystem fs, | |
* @return true if the file is created, otherwise the file exists. | ||
* @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.
Show resolved
Hide resolved
|
||
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,7 @@ 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."); | ||
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. We changed the behavior here? In the past it could return false but now it will throw an exception? 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. All these 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. OK, so let's add more javadoc to say it is only used when restoring snapshot, so later people will not misuse it. |
||
} | ||
|
||
/** | ||
|
@@ -464,7 +448,7 @@ public static boolean createFromHFileLink(final Configuration conf, final FileSy | |
* @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, | ||
public static String createFromHFileLink(final Configuration conf, final FileSystem fs, | ||
wchevreuil marked this conversation as resolved.
Show resolved
Hide resolved
|
||
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 |
---|---|---|
|
@@ -453,56 +453,24 @@ private List<RegionInfo> createFsLayout( | |
List<RegionInfo> newRegions, | ||
final CreateHdfsRegions hdfsRegionHandler) throws IOException { | ||
final MasterFileSystem mfs = env.getMasterServices().getMasterFileSystem(); | ||
final Path tempdir = mfs.getTempDir(); | ||
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? 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.
Show resolved
Hide resolved
|
||
} | ||
((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; | ||
} | ||
|
||
/** | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Where do we use this class?