You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@hbase.apache.org by GitBox <gi...@apache.org> on 2020/05/29 15:07:52 UTC
[GitHub] [hbase] Apache9 opened a new pull request #1806: HBASE-24471 The way we bootstrap meta table is confusing
Apache9 opened a new pull request #1806:
URL: https://github.com/apache/hbase/pull/1806
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [hbase] Apache-HBase commented on pull request #1806: HBASE-24471 The way we bootstrap meta table is confusing
Posted by GitBox <gi...@apache.org>.
Apache-HBase commented on pull request #1806:
URL: https://github.com/apache/hbase/pull/1806#issuecomment-636268338
:confetti_ball: **+1 overall**
| Vote | Subsystem | Runtime | Comment |
|:----:|----------:|--------:|:--------|
| +0 :ok: | reexec | 1m 29s | Docker mode activated. |
| -0 :warning: | yetus | 0m 4s | Unprocessed flag(s): --brief-report-file --spotbugs-strict-precheck --whitespace-eol-ignore-list --whitespace-tabs-ignore-list --quick-hadoopcheck |
||| _ Prechecks _ |
||| _ master Compile Tests _ |
| +0 :ok: | mvndep | 0m 23s | Maven dependency ordering for branch |
| +1 :green_heart: | mvninstall | 4m 15s | master passed |
| +1 :green_heart: | compile | 2m 1s | master passed |
| +1 :green_heart: | shadedjars | 5m 47s | branch has no errors when building our shaded downstream artifacts. |
| -0 :warning: | javadoc | 0m 41s | hbase-server in master failed. |
||| _ Patch Compile Tests _ |
| +0 :ok: | mvndep | 0m 15s | Maven dependency ordering for patch |
| +1 :green_heart: | mvninstall | 4m 2s | the patch passed |
| +1 :green_heart: | compile | 2m 6s | the patch passed |
| +1 :green_heart: | javac | 2m 6s | the patch passed |
| +1 :green_heart: | shadedjars | 5m 43s | patch has no errors when building our shaded downstream artifacts. |
| -0 :warning: | javadoc | 0m 39s | hbase-server in the patch failed. |
||| _ Other Tests _ |
| +1 :green_heart: | unit | 0m 58s | hbase-protocol-shaded in the patch passed. |
| +1 :green_heart: | unit | 126m 17s | hbase-server in the patch passed. |
| | | 157m 32s | |
| Subsystem | Report/Notes |
|----------:|:-------------|
| Docker | Client=19.03.10 Server=19.03.10 base: https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-1806/2/artifact/yetus-jdk11-hadoop3-check/output/Dockerfile |
| GITHUB PR | https://github.com/apache/hbase/pull/1806 |
| Optional Tests | unit javac javadoc shadedjars compile |
| uname | Linux 93fef3dd42e5 4.15.0-60-generic #67-Ubuntu SMP Thu Aug 22 16:55:30 UTC 2019 x86_64 x86_64 x86_64 GNU/Linux |
| Build tool | maven |
| Personality | dev-support/hbase-personality.sh |
| git revision | master / 78fce0f333 |
| Default Java | 2020-01-14 |
| javadoc | https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-1806/2/artifact/yetus-jdk11-hadoop3-check/output/branch-javadoc-hbase-server.txt |
| javadoc | https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-1806/2/artifact/yetus-jdk11-hadoop3-check/output/patch-javadoc-hbase-server.txt |
| Test Results | https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-1806/2/testReport/ |
| Max. process+thread count | 4565 (vs. ulimit of 12500) |
| modules | C: hbase-protocol-shaded hbase-server U: . |
| Console output | https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-1806/2/console |
| versions | git=2.17.1 maven=(cecedd343002696d0abb50b32b541b8a6ba2883f) |
| Powered by | Apache Yetus 0.11.1 https://yetus.apache.org |
This message was automatically generated.
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [hbase] Apache-HBase commented on pull request #1806: HBASE-24471 The way we bootstrap meta table is confusing
Posted by GitBox <gi...@apache.org>.
Apache-HBase commented on pull request #1806:
URL: https://github.com/apache/hbase/pull/1806#issuecomment-636100105
:confetti_ball: **+1 overall**
| Vote | Subsystem | Runtime | Comment |
|:----:|----------:|--------:|:--------|
| +0 :ok: | reexec | 0m 30s | Docker mode activated. |
| -0 :warning: | yetus | 0m 3s | Unprocessed flag(s): --brief-report-file --spotbugs-strict-precheck --whitespace-eol-ignore-list --whitespace-tabs-ignore-list --quick-hadoopcheck |
||| _ Prechecks _ |
||| _ master Compile Tests _ |
| +0 :ok: | mvndep | 0m 21s | Maven dependency ordering for branch |
| +1 :green_heart: | mvninstall | 4m 8s | master passed |
| +1 :green_heart: | compile | 1m 59s | master passed |
| +1 :green_heart: | shadedjars | 5m 46s | branch has no errors when building our shaded downstream artifacts. |
| -0 :warning: | javadoc | 0m 39s | hbase-server in master failed. |
||| _ Patch Compile Tests _ |
| +0 :ok: | mvndep | 0m 15s | Maven dependency ordering for patch |
| +1 :green_heart: | mvninstall | 3m 59s | the patch passed |
| +1 :green_heart: | compile | 2m 0s | the patch passed |
| +1 :green_heart: | javac | 2m 0s | the patch passed |
| +1 :green_heart: | shadedjars | 5m 45s | patch has no errors when building our shaded downstream artifacts. |
| -0 :warning: | javadoc | 0m 38s | hbase-server in the patch failed. |
||| _ Other Tests _ |
| +1 :green_heart: | unit | 0m 58s | hbase-protocol-shaded in the patch passed. |
| +1 :green_heart: | unit | 124m 19s | hbase-server in the patch passed. |
| | | 154m 8s | |
| Subsystem | Report/Notes |
|----------:|:-------------|
| Docker | Client=19.03.10 Server=19.03.10 base: https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-1806/1/artifact/yetus-jdk11-hadoop3-check/output/Dockerfile |
| GITHUB PR | https://github.com/apache/hbase/pull/1806 |
| Optional Tests | unit javac javadoc shadedjars compile |
| uname | Linux a8e7d3a83c68 4.15.0-58-generic #64-Ubuntu SMP Tue Aug 6 11:12:41 UTC 2019 x86_64 x86_64 x86_64 GNU/Linux |
| Build tool | maven |
| Personality | dev-support/hbase-personality.sh |
| git revision | master / da2e03bb3b |
| Default Java | 2020-01-14 |
| javadoc | https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-1806/1/artifact/yetus-jdk11-hadoop3-check/output/branch-javadoc-hbase-server.txt |
| javadoc | https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-1806/1/artifact/yetus-jdk11-hadoop3-check/output/patch-javadoc-hbase-server.txt |
| Test Results | https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-1806/1/testReport/ |
| Max. process+thread count | 4436 (vs. ulimit of 12500) |
| modules | C: hbase-protocol-shaded hbase-server U: . |
| Console output | https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-1806/1/console |
| versions | git=2.17.1 maven=(cecedd343002696d0abb50b32b541b8a6ba2883f) |
| Powered by | Apache Yetus 0.11.1 https://yetus.apache.org |
This message was automatically generated.
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [hbase] Apache-HBase commented on pull request #1806: HBASE-24471 The way we bootstrap meta table is confusing
Posted by GitBox <gi...@apache.org>.
Apache-HBase commented on pull request #1806:
URL: https://github.com/apache/hbase/pull/1806#issuecomment-636269998
:confetti_ball: **+1 overall**
| Vote | Subsystem | Runtime | Comment |
|:----:|----------:|--------:|:--------|
| +0 :ok: | reexec | 0m 43s | Docker mode activated. |
| -0 :warning: | yetus | 0m 3s | Unprocessed flag(s): --brief-report-file --spotbugs-strict-precheck --whitespace-eol-ignore-list --whitespace-tabs-ignore-list --quick-hadoopcheck |
||| _ Prechecks _ |
||| _ master Compile Tests _ |
| +0 :ok: | mvndep | 0m 22s | Maven dependency ordering for branch |
| +1 :green_heart: | mvninstall | 4m 14s | master passed |
| +1 :green_heart: | compile | 1m 49s | master passed |
| +1 :green_heart: | shadedjars | 6m 37s | branch has no errors when building our shaded downstream artifacts. |
| +1 :green_heart: | javadoc | 0m 54s | master passed |
||| _ Patch Compile Tests _ |
| +0 :ok: | mvndep | 0m 14s | Maven dependency ordering for patch |
| +1 :green_heart: | mvninstall | 3m 56s | the patch passed |
| +1 :green_heart: | compile | 1m 53s | the patch passed |
| +1 :green_heart: | javac | 1m 53s | the patch passed |
| +1 :green_heart: | shadedjars | 6m 45s | patch has no errors when building our shaded downstream artifacts. |
| +1 :green_heart: | javadoc | 0m 52s | the patch passed |
||| _ Other Tests _ |
| +1 :green_heart: | unit | 0m 48s | hbase-protocol-shaded in the patch passed. |
| +1 :green_heart: | unit | 144m 15s | hbase-server in the patch passed. |
| | | 175m 43s | |
| Subsystem | Report/Notes |
|----------:|:-------------|
| Docker | Client=19.03.10 Server=19.03.10 base: https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-1806/2/artifact/yetus-jdk8-hadoop3-check/output/Dockerfile |
| GITHUB PR | https://github.com/apache/hbase/pull/1806 |
| Optional Tests | unit javac javadoc shadedjars compile |
| uname | Linux 82338eabb7f2 4.15.0-58-generic #64-Ubuntu SMP Tue Aug 6 11:12:41 UTC 2019 x86_64 x86_64 x86_64 GNU/Linux |
| Build tool | maven |
| Personality | dev-support/hbase-personality.sh |
| git revision | master / 78fce0f333 |
| Default Java | 1.8.0_232 |
| Test Results | https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-1806/2/testReport/ |
| Max. process+thread count | 4533 (vs. ulimit of 12500) |
| modules | C: hbase-protocol-shaded hbase-server U: . |
| Console output | https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-1806/2/console |
| versions | git=2.17.1 maven=(cecedd343002696d0abb50b32b541b8a6ba2883f) |
| Powered by | Apache Yetus 0.11.1 https://yetus.apache.org |
This message was automatically generated.
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [hbase] virajjasani commented on a change in pull request #1806: HBASE-24471 The way we bootstrap meta table is confusing
Posted by GitBox <gi...@apache.org>.
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:
users@infra.apache.org
[GitHub] [hbase] Apache-HBase commented on pull request #1806: HBASE-24471 The way we bootstrap meta table is confusing
Posted by GitBox <gi...@apache.org>.
Apache-HBase commented on pull request #1806:
URL: https://github.com/apache/hbase/pull/1806#issuecomment-636104082
:confetti_ball: **+1 overall**
| Vote | Subsystem | Runtime | Comment |
|:----:|----------:|--------:|:--------|
| +0 :ok: | reexec | 0m 33s | Docker mode activated. |
| -0 :warning: | yetus | 0m 3s | Unprocessed flag(s): --brief-report-file --spotbugs-strict-precheck --whitespace-eol-ignore-list --whitespace-tabs-ignore-list --quick-hadoopcheck |
||| _ Prechecks _ |
||| _ master Compile Tests _ |
| +0 :ok: | mvndep | 0m 22s | Maven dependency ordering for branch |
| +1 :green_heart: | mvninstall | 3m 39s | master passed |
| +1 :green_heart: | compile | 1m 43s | master passed |
| +1 :green_heart: | shadedjars | 5m 55s | branch has no errors when building our shaded downstream artifacts. |
| +1 :green_heart: | javadoc | 0m 49s | master passed |
||| _ Patch Compile Tests _ |
| +0 :ok: | mvndep | 0m 15s | Maven dependency ordering for patch |
| +1 :green_heart: | mvninstall | 3m 22s | the patch passed |
| +1 :green_heart: | compile | 1m 41s | the patch passed |
| +1 :green_heart: | javac | 1m 41s | the patch passed |
| +1 :green_heart: | shadedjars | 5m 39s | patch has no errors when building our shaded downstream artifacts. |
| +1 :green_heart: | javadoc | 0m 52s | the patch passed |
||| _ Other Tests _ |
| +1 :green_heart: | unit | 0m 48s | hbase-protocol-shaded in the patch passed. |
| +1 :green_heart: | unit | 135m 17s | hbase-server in the patch passed. |
| | | 162m 59s | |
| Subsystem | Report/Notes |
|----------:|:-------------|
| Docker | Client=19.03.10 Server=19.03.10 base: https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-1806/1/artifact/yetus-jdk8-hadoop3-check/output/Dockerfile |
| GITHUB PR | https://github.com/apache/hbase/pull/1806 |
| Optional Tests | unit javac javadoc shadedjars compile |
| uname | Linux 88722deb863a 4.15.0-58-generic #64-Ubuntu SMP Tue Aug 6 11:12:41 UTC 2019 x86_64 x86_64 x86_64 GNU/Linux |
| Build tool | maven |
| Personality | dev-support/hbase-personality.sh |
| git revision | master / da2e03bb3b |
| Default Java | 1.8.0_232 |
| Test Results | https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-1806/1/testReport/ |
| Max. process+thread count | 4430 (vs. ulimit of 12500) |
| modules | C: hbase-protocol-shaded hbase-server U: . |
| Console output | https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-1806/1/console |
| versions | git=2.17.1 maven=(cecedd343002696d0abb50b32b541b8a6ba2883f) |
| Powered by | Apache Yetus 0.11.1 https://yetus.apache.org |
This message was automatically generated.
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [hbase] Apache9 merged pull request #1806: HBASE-24471 The way we bootstrap meta table is confusing
Posted by GitBox <gi...@apache.org>.
Apache9 merged pull request #1806:
URL: https://github.com/apache/hbase/pull/1806
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [hbase] Apache-HBase commented on pull request #1806: HBASE-24471 The way we bootstrap meta table is confusing
Posted by GitBox <gi...@apache.org>.
Apache-HBase commented on pull request #1806:
URL: https://github.com/apache/hbase/pull/1806#issuecomment-636254732
:confetti_ball: **+1 overall**
| Vote | Subsystem | Runtime | Comment |
|:----:|----------:|--------:|:--------|
| +0 :ok: | reexec | 0m 37s | Docker mode activated. |
||| _ Prechecks _ |
| +1 :green_heart: | dupname | 0m 0s | No case conflicting files found. |
| +0 :ok: | prototool | 0m 0s | prototool was not available. |
| +1 :green_heart: | hbaseanti | 0m 0s | Patch does not have any anti-patterns. |
| +1 :green_heart: | @author | 0m 0s | The patch does not contain any @author tags. |
||| _ master Compile Tests _ |
| +0 :ok: | mvndep | 0m 23s | Maven dependency ordering for branch |
| +1 :green_heart: | mvninstall | 4m 9s | master passed |
| +1 :green_heart: | checkstyle | 1m 23s | master passed |
| +1 :green_heart: | spotbugs | 6m 2s | master passed |
||| _ Patch Compile Tests _ |
| +0 :ok: | mvndep | 0m 14s | Maven dependency ordering for patch |
| +1 :green_heart: | mvninstall | 3m 28s | the patch passed |
| +1 :green_heart: | checkstyle | 0m 11s | The patch passed checkstyle in hbase-protocol-shaded |
| +1 :green_heart: | checkstyle | 1m 3s | hbase-server: The patch generated 0 new + 66 unchanged - 4 fixed = 66 total (was 70) |
| +1 :green_heart: | whitespace | 0m 0s | The patch has no whitespace issues. |
| +1 :green_heart: | hadoopcheck | 11m 10s | Patch does not cause any errors with Hadoop 3.1.2 3.2.1. |
| +1 :green_heart: | hbaseprotoc | 1m 33s | the patch passed |
| +1 :green_heart: | spotbugs | 5m 31s | the patch passed |
||| _ Other Tests _ |
| +1 :green_heart: | asflicense | 0m 26s | The patch does not generate ASF License warnings. |
| | | 43m 39s | |
| Subsystem | Report/Notes |
|----------:|:-------------|
| Docker | Client=19.03.10 Server=19.03.10 base: https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-1806/2/artifact/yetus-general-check/output/Dockerfile |
| GITHUB PR | https://github.com/apache/hbase/pull/1806 |
| Optional Tests | dupname asflicense cc hbaseprotoc prototool spotbugs hadoopcheck hbaseanti checkstyle |
| uname | Linux 2aa027b4c944 4.15.0-58-generic #64-Ubuntu SMP Tue Aug 6 11:12:41 UTC 2019 x86_64 x86_64 x86_64 GNU/Linux |
| Build tool | maven |
| Personality | dev-support/hbase-personality.sh |
| git revision | master / 78fce0f333 |
| Max. process+thread count | 94 (vs. ulimit of 12500) |
| modules | C: hbase-protocol-shaded hbase-server U: . |
| Console output | https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-1806/2/console |
| versions | git=2.17.1 maven=(cecedd343002696d0abb50b32b541b8a6ba2883f) spotbugs=3.1.12 |
| Powered by | Apache Yetus 0.11.1 https://yetus.apache.org |
This message was automatically generated.
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [hbase] Apache9 commented on a change in pull request #1806: HBASE-24471 The way we bootstrap meta table is confusing
Posted by GitBox <gi...@apache.org>.
Apache9 commented on a change in pull request #1806:
URL: https://github.com/apache/hbase/pull/1806#discussion_r432786009
##########
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:
+ Configuration conf = env.getMasterConfiguration();
+ Path rootDir = CommonFSUtils.getRootDir(conf);
+ writeFsLayout(rootDir, conf);
+ setNextState(InitMetaState.INIT_META_ASSIGN_META);
+ return Flow.HAS_MORE_STATE;
+ case INIT_META_ASSIGN_META:
+ LOG.info("Going to assign meta");
+ addChildProcedure(env.getAssignmentManager()
Review comment:
You misunderstood the goal here...
The procedure is 'InitMetaProcedure', not 'InitRootDirProcedure', it is just for initializaing meta table. The code for initializing the root dir is still in MasterFileSystem, what you have reviewed above...
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [hbase] Apache9 commented on a change in pull request #1806: HBASE-24471 The way we bootstrap meta table is confusing
Posted by GitBox <gi...@apache.org>.
Apache9 commented on a change in pull request #1806:
URL: https://github.com/apache/hbase/pull/1806#discussion_r432785688
##########
File path: hbase-protocol-shaded/src/main/protobuf/server/master/MasterProcedure.proto
##########
@@ -489,8 +489,9 @@ message ReopenTableRegionsStateData {
}
enum InitMetaState {
- INIT_META_ASSIGN_META = 1;
- INIT_META_CREATE_NAMESPACES = 2;
+ INIT_META_WRITE_FS_LAYOUT = 1;
Review comment:
I tried to do this in the patch, i.e, adding 'INIT_META_WRITE_FS_LAYOUT = 3', but finally I found that it is not necessary, as changing the order or adding a new enum will both break the compatibility, so let's just kep the code clean, i.e, the order in the enum is the order when executing.
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [hbase] Apache-HBase commented on pull request #1806: HBASE-24471 The way we bootstrap meta table is confusing
Posted by GitBox <gi...@apache.org>.
Apache-HBase commented on pull request #1806:
URL: https://github.com/apache/hbase/pull/1806#issuecomment-636047587
:confetti_ball: **+1 overall**
| Vote | Subsystem | Runtime | Comment |
|:----:|----------:|--------:|:--------|
| +0 :ok: | reexec | 0m 31s | Docker mode activated. |
||| _ Prechecks _ |
| +1 :green_heart: | dupname | 0m 0s | No case conflicting files found. |
| +0 :ok: | prototool | 0m 0s | prototool was not available. |
| +1 :green_heart: | hbaseanti | 0m 0s | Patch does not have any anti-patterns. |
| +1 :green_heart: | @author | 0m 0s | The patch does not contain any @author tags. |
||| _ master Compile Tests _ |
| +0 :ok: | mvndep | 0m 23s | Maven dependency ordering for branch |
| +1 :green_heart: | mvninstall | 3m 37s | master passed |
| +1 :green_heart: | checkstyle | 1m 17s | master passed |
| +1 :green_heart: | spotbugs | 5m 2s | master passed |
||| _ Patch Compile Tests _ |
| +0 :ok: | mvndep | 0m 13s | Maven dependency ordering for patch |
| +1 :green_heart: | mvninstall | 3m 19s | the patch passed |
| +1 :green_heart: | checkstyle | 0m 10s | The patch passed checkstyle in hbase-protocol-shaded |
| +1 :green_heart: | checkstyle | 1m 6s | hbase-server: The patch generated 0 new + 66 unchanged - 4 fixed = 66 total (was 70) |
| +1 :green_heart: | whitespace | 0m 0s | The patch has no whitespace issues. |
| +1 :green_heart: | hadoopcheck | 11m 9s | Patch does not cause any errors with Hadoop 3.1.2 3.2.1. |
| +1 :green_heart: | hbaseprotoc | 1m 36s | the patch passed |
| +1 :green_heart: | spotbugs | 5m 26s | the patch passed |
||| _ Other Tests _ |
| +1 :green_heart: | asflicense | 0m 26s | The patch does not generate ASF License warnings. |
| | | 41m 39s | |
| Subsystem | Report/Notes |
|----------:|:-------------|
| Docker | Client=19.03.10 Server=19.03.10 base: https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-1806/1/artifact/yetus-general-check/output/Dockerfile |
| GITHUB PR | https://github.com/apache/hbase/pull/1806 |
| Optional Tests | dupname asflicense cc hbaseprotoc prototool spotbugs hadoopcheck hbaseanti checkstyle |
| uname | Linux 2b7e9234f926 4.15.0-60-generic #67-Ubuntu SMP Thu Aug 22 16:55:30 UTC 2019 x86_64 x86_64 x86_64 GNU/Linux |
| Build tool | maven |
| Personality | dev-support/hbase-personality.sh |
| git revision | master / da2e03bb3b |
| Max. process+thread count | 94 (vs. ulimit of 12500) |
| modules | C: hbase-protocol-shaded hbase-server U: . |
| Console output | https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-1806/1/console |
| versions | git=2.17.1 maven=(cecedd343002696d0abb50b32b541b8a6ba2883f) spotbugs=3.1.12 |
| Powered by | Apache Yetus 0.11.1 https://yetus.apache.org |
This message was automatically generated.
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [hbase] ndimiduk commented on a change in pull request #1806: HBASE-24471 The way we bootstrap meta table is confusing
Posted by GitBox <gi...@apache.org>.
ndimiduk commented on a change in pull request #1806:
URL: https://github.com/apache/hbase/pull/1806#discussion_r432767573
##########
File path: hbase-protocol-shaded/src/main/protobuf/server/master/MasterProcedure.proto
##########
@@ -489,8 +489,9 @@ message ReopenTableRegionsStateData {
}
enum InitMetaState {
- INIT_META_ASSIGN_META = 1;
- INIT_META_CREATE_NAMESPACES = 2;
+ INIT_META_WRITE_FS_LAYOUT = 1;
Review comment:
Why not preserve the existing ordinals? The state machine implemented over this enum can control the transitions however it sees fit, and you'll can keep serialized data that much more backwards-compatible.
Unless these existing states have new meanings in the new paradigm...
##########
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");
Review comment:
How about `"Configured '" + HConstants.HBASE_DIR + "' + rd + " is not a directory."` ?
##########
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:
+ Configuration conf = env.getMasterConfiguration();
+ Path rootDir = CommonFSUtils.getRootDir(conf);
+ writeFsLayout(rootDir, conf);
+ setNextState(InitMetaState.INIT_META_ASSIGN_META);
+ return Flow.HAS_MORE_STATE;
+ case INIT_META_ASSIGN_META:
+ LOG.info("Going to assign meta");
+ addChildProcedure(env.getAssignmentManager()
Review comment:
Oh, it is because you use `skipPersistence()` down below? The `ProcedureExecutor` and `ProcedureStore` can function without the root filesystem being available?
##########
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:
+ Configuration conf = env.getMasterConfiguration();
+ Path rootDir = CommonFSUtils.getRootDir(conf);
+ writeFsLayout(rootDir, conf);
+ setNextState(InitMetaState.INIT_META_ASSIGN_META);
+ return Flow.HAS_MORE_STATE;
+ case INIT_META_ASSIGN_META:
+ LOG.info("Going to assign meta");
+ addChildProcedure(env.getAssignmentManager()
Review comment:
The procedures are stored in the `LocalStore`/`LocalRegion`. The `LocalRegion` is backed by `${hbase.rootdir}/MasterData`. How can we have a procedure that initializes `hbase.rootdir`? Isn't this a circular dependency?
##########
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(
Review comment:
I know it's not your code, but mind unpacking these inline configuration lookups? They're terribly illegible.
##########
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:
Indeed, static analysis tells me that this method's return value is never used. Perhaps this is the legacy at some attempt at a fluent style?
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [hbase] Apache9 commented on pull request #1806: HBASE-24471 The way we bootstrap meta table is confusing
Posted by GitBox <gi...@apache.org>.
Apache9 commented on pull request #1806:
URL: https://github.com/apache/hbase/pull/1806#issuecomment-636028120
The current patch will lead to an 'incompatible change', no doubt, but I think it is fine. The InitMetaProcedure is only used when bootstraping a cluster. Once it is done, we will never use it again. So it is not likely to introduce any problems when rolling upgrading an existing cluster in real world. And even if someone does upgrade the version during bootstraping, since there is no actual data in the cluster before we finish InitMetaProcedure, the user ccan just cleanup the cluster and re-deploy again, there will be no data loss.
----------------------------------------------------------------
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:
users@infra.apache.org