virajjasani commented on a change in pull request #1806:
URL: https://github.com/apache/hbase/pull/1806#discussion_r432677250
##########
File path:
hbase-server/src/main/java/org/apache/hadoop/hbase/master/MasterFileSystem.java
##########
@@ -245,61 +238,56 @@ public ClusterId getClusterId() {
* directory with necessary bootup files).
*/
private Path checkRootDir(final Path rd, final Configuration c, final
FileSystem fs)
- throws IOException {
+ throws IOException {
// If FS is in safe mode wait till out of it.
FSUtils.waitOnSafeMode(c, c.getInt(HConstants.THREAD_WAKE_FREQUENCY, 10 *
1000));
// Filesystem is good. Go ahead and check for hbase.rootdir.
+ FileStatus status;
+ try {
+ status = fs.getFileStatus(rd);
+ } catch (FileNotFoundException e) {
+ status = null;
+ }
try {
- if (!fs.exists(rd)) {
- fs.mkdirs(rd);
+ if (status == null) {
+ if (!fs.mkdirs(rd)) {
+ throw new IOException("Can not create root dir: " + rd);
+ }
// DFS leaves safe mode with 0 DNs when there are 0 blocks.
// We used to handle this by checking the current DN count and waiting
until
// it is nonzero. With security, the check for datanode count doesn't
work --
// it is a privileged op. So instead we adopt the strategy of the
jobtracker
// and simply retry file creation during bootstrap indefinitely. As
soon as
// there is one datanode it will succeed. Permission problems should
have
// already been caught by mkdirs above.
- FSUtils.setVersion(fs, rd, c.getInt(HConstants.THREAD_WAKE_FREQUENCY,
- 10 * 1000), c.getInt(HConstants.VERSION_FILE_WRITE_ATTEMPTS,
- HConstants.DEFAULT_VERSION_FILE_WRITE_ATTEMPTS));
+ FSUtils.setVersion(fs, rd, c.getInt(HConstants.THREAD_WAKE_FREQUENCY,
10 * 1000), c.getInt(
+ HConstants.VERSION_FILE_WRITE_ATTEMPTS,
HConstants.DEFAULT_VERSION_FILE_WRITE_ATTEMPTS));
} else {
- if (!fs.isDirectory(rd)) {
- throw new IllegalArgumentException(rd.toString() + " is not a
directory");
+ if (!status.isDirectory()) {
+ throw new IllegalArgumentException(rd + " is not a directory");
}
// as above
- FSUtils.checkVersion(fs, rd, true,
c.getInt(HConstants.THREAD_WAKE_FREQUENCY,
- 10 * 1000), c.getInt(HConstants.VERSION_FILE_WRITE_ATTEMPTS,
+ FSUtils.checkVersion(fs, rd, true,
c.getInt(HConstants.THREAD_WAKE_FREQUENCY, 10 * 1000),
+ c.getInt(HConstants.VERSION_FILE_WRITE_ATTEMPTS,
HConstants.DEFAULT_VERSION_FILE_WRITE_ATTEMPTS));
}
} catch (DeserializationException de) {
- LOG.error(HBaseMarkers.FATAL, "Please fix invalid configuration for "
- + HConstants.HBASE_DIR, de);
+ LOG.error(HBaseMarkers.FATAL, "Please fix invalid configuration for " +
HConstants.HBASE_DIR,
+ de);
throw new IOException(de);
} catch (IllegalArgumentException iae) {
- LOG.error(HBaseMarkers.FATAL, "Please fix invalid configuration for "
- + HConstants.HBASE_DIR + " " + rd.toString(), iae);
+ LOG.error(HBaseMarkers.FATAL,
+ "Please fix invalid configuration for " + HConstants.HBASE_DIR + " " +
rd.toString(), iae);
Review comment:
Would you like to use placeholders `{}` for HConstants.HBASE_DIR and
rd.toString()? Also, only `rd` should suffice.
##########
File path:
hbase-server/src/main/java/org/apache/hadoop/hbase/master/MasterFileSystem.java
##########
@@ -245,61 +238,56 @@ public ClusterId getClusterId() {
* directory with necessary bootup files).
*/
private Path checkRootDir(final Path rd, final Configuration c, final
FileSystem fs)
Review comment:
The purpose of this method is to check if rootdir exists right? If it
can't get the Path obj, it throws Exception. If config for rootdir is not
valid, it throws Exception. However, after assigning `clusterId` of the object,
we don't really need to return path `rd` (hopefully, it's not getting mutated
internally) which was passed in the argument already. I believe this method
should be `private void checkRootDir()`. WDYT?
##########
File path:
hbase-server/src/main/java/org/apache/hadoop/hbase/master/procedure/InitMetaProcedure.java
##########
@@ -64,41 +72,68 @@ public TableOperationType getTableOperationType() {
return TableOperationType.CREATE;
}
+ private static void writeFsLayout(Path rootDir, Configuration conf) throws
IOException {
+ LOG.info("BOOTSTRAP: creating hbase:meta region");
+ FileSystem fs = rootDir.getFileSystem(conf);
+ Path tableDir = CommonFSUtils.getTableDir(rootDir,
TableName.META_TABLE_NAME);
+ if (fs.exists(tableDir) && !fs.delete(tableDir, true)) {
+ LOG.warn("Can not delete partial created meta table, continue...");
+ }
+ // Bootstrapping, make sure blockcache is off. Else, one will be
+ // created here in bootstrap and it'll need to be cleaned up. Better to
+ // not make it in first place. Turn off block caching for bootstrap.
+ // Enable after.
+ FSTableDescriptors.tryUpdateMetaTableDescriptor(conf, fs, rootDir,
+ builder -> builder.setRegionReplication(
+ conf.getInt(HConstants.META_REPLICAS_NUM,
HConstants.DEFAULT_META_REPLICA_NUM)));
+ TableDescriptor metaDescriptor = new
FSTableDescriptors(conf).get(TableName.META_TABLE_NAME);
+ HRegion
+ .createHRegion(RegionInfoBuilder.FIRST_META_REGIONINFO, rootDir, conf,
metaDescriptor, null)
+ .close();
+ }
+
@Override
protected Flow executeFromState(MasterProcedureEnv env, InitMetaState state)
- throws ProcedureSuspendedException, ProcedureYieldException,
InterruptedException {
+ throws ProcedureSuspendedException, ProcedureYieldException,
InterruptedException {
LOG.debug("Execute {}", this);
- switch (state) {
- case INIT_META_ASSIGN_META:
- LOG.info("Going to assign meta");
- addChildProcedure(env.getAssignmentManager()
-
.createAssignProcedures(Arrays.asList(RegionInfoBuilder.FIRST_META_REGIONINFO)));
- setNextState(InitMetaState.INIT_META_CREATE_NAMESPACES);
- return Flow.HAS_MORE_STATE;
- case INIT_META_CREATE_NAMESPACES:
- LOG.info("Going to create {} and {} namespaces", DEFAULT_NAMESPACE,
SYSTEM_NAMESPACE);
- try {
+ try {
+ switch (state) {
+ case INIT_META_WRITE_FS_LAYOUT:
Review comment:
Ok so performing bootstrap as part of the Procedure is the major change.
Seems applicable to fresh clusters. 👍
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
[email protected]
