Skip to content

Conversation

@pankaj72981
Copy link
Contributor

HMaste should validate the meta table descriptor during startup and rewrite if any mismatch, meanwhile RegionServer should read the default meta table descriptor to avoid inconsistencies.

@Apache-HBase
Copy link

🎊 +1 overall

Vote Subsystem Runtime Comment
+0 🆗 reexec 6m 38s Docker mode activated.
_ Prechecks _
+1 💚 dupname 0m 0s No case conflicting files found.
+1 💚 hbaseanti 0m 0s Patch does not have any anti-patterns.
+1 💚 @author 0m 0s The patch does not contain any @author tags.
_ master Compile Tests _
+0 🆗 mvndep 0m 20s Maven dependency ordering for branch
+1 💚 mvninstall 4m 14s master passed
+1 💚 compile 4m 3s master passed
+1 💚 checkstyle 1m 35s master passed
+1 💚 spotbugs 2m 56s master passed
_ Patch Compile Tests _
+0 🆗 mvndep 0m 11s Maven dependency ordering for patch
+1 💚 mvninstall 4m 1s the patch passed
+1 💚 compile 4m 6s the patch passed
+1 💚 javac 4m 6s the patch passed
+1 💚 checkstyle 1m 32s the patch passed
+1 💚 whitespace 0m 0s The patch has no whitespace issues.
+1 💚 hadoopcheck 21m 18s Patch does not cause any errors with Hadoop 3.1.2 3.2.1 3.3.0.
+1 💚 spotbugs 3m 56s the patch passed
_ Other Tests _
+1 💚 asflicense 0m 28s The patch does not generate ASF License warnings.
64m 17s
Subsystem Report/Notes
Docker ClientAPI=1.41 ServerAPI=1.41 base: https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-3287/1/artifact/yetus-general-check/output/Dockerfile
GITHUB PR #3287
Optional Tests dupname asflicense javac spotbugs hadoopcheck hbaseanti checkstyle compile
uname Linux 8288ddba4355 4.15.0-128-generic #131-Ubuntu SMP Wed Dec 9 06:57:35 UTC 2020 x86_64 x86_64 x86_64 GNU/Linux
Build tool maven
Personality dev-support/hbase-personality.sh
git revision master / 7c24ed4
Default Java AdoptOpenJDK-1.8.0_282-b08
Max. process+thread count 86 (vs. ulimit of 30000)
modules C: hbase-common hbase-server U: .
Console output https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-3287/1/console
versions git=2.17.1 maven=3.6.3 spotbugs=4.2.2
Powered by Apache Yetus 0.12.0 https://yetus.apache.org

This message was automatically generated.

* List of column families that cannot be deleted from the hbase:meta table.
* They are critical to cluster operation. This is a bit of an odd place to
* keep this list but then this is the tooling that does add/remove. Keeping
* it local!
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This comment is wrong, right? We don't have any tooling in HConstants.

This should go where table descriptors are defined and managed.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok, will move it back to ModifyTableProcedure


// Default meta table descriptor, will be used by RegionServer during rolling upgrade until
// HMaster write latest 2.x meta table descriptor
private TableDescriptor defaultMetaTableDesc = null;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What happens when using this "default descriptor" the regionserver attempts to update meta? Can that happen? We are assuming the master will rewrite soon, but is that valid? Regionservers don't run masters, they rely on an operator to do that. Who knows what the operator is doing.

Would it be better to address specific fallbacks where something is missing or not expected?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

RegionServer can't update the meta descriptor,
org.apache.hadoop.hbase.regionserver.HRegionServer#canUpdateTableDescriptor

TableDescriptor td = getTableDescriptorFromFs(fs, rootdir, TableName.META_TABLE_NAME);
validateMetaTableDescriptor(td);
return td;
} catch (TableInfoMissingException | NoSuchColumnFamilyException e) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This makes sense. The table might be missing altogether (what current code handles), or might be missing a column family due to legacy (what this change handles)

} catch (TableInfoMissingException | NoSuchColumnFamilyException e) {
// Meta is still in old format, return the default meta table descriptor util we have meta
// descriptor in HBase 2.x format
return defaultMetaTableDesc;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This seems wrong.

createMetaTableDescriptorBuilder should return a builder for what the current version of the code expects.

If we need fallbacks to ride over an upgrade case, those fallbacks should be implemented where the errors are happening.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This seems wrong.

createMetaTableDescriptorBuilder should return a builder for what the current version of the code expects.

If we need fallbacks to ride over an upgrade case, those fallbacks should be implemented where the errors are happening.

In the current version of code createMetaTableDescriptorBuilder is used only during HMaster startup via InitMetaProcedure when table info file is missing.
Agree it is a hack (defaultMetaTableDesc = null), where RegionServer will get the default meta descriptor (createMetaTableDescriptorBuilder) until HMaster rewrite the proper one.

@Apache-HBase
Copy link

🎊 +1 overall

Vote Subsystem Runtime Comment
+0 🆗 reexec 0m 27s Docker mode activated.
-0 ⚠️ 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 🆗 mvndep 0m 23s Maven dependency ordering for branch
+1 💚 mvninstall 4m 24s master passed
+1 💚 compile 1m 38s master passed
+1 💚 shadedjars 8m 7s branch has no errors when building our shaded downstream artifacts.
+1 💚 javadoc 1m 7s master passed
_ Patch Compile Tests _
+0 🆗 mvndep 0m 16s Maven dependency ordering for patch
+1 💚 mvninstall 4m 15s the patch passed
+1 💚 compile 1m 36s the patch passed
+1 💚 javac 1m 36s the patch passed
+1 💚 shadedjars 8m 5s patch has no errors when building our shaded downstream artifacts.
+1 💚 javadoc 1m 3s the patch passed
_ Other Tests _
+1 💚 unit 2m 0s hbase-common in the patch passed.
+1 💚 unit 139m 22s hbase-server in the patch passed.
175m 23s
Subsystem Report/Notes
Docker ClientAPI=1.41 ServerAPI=1.41 base: https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-3287/1/artifact/yetus-jdk11-hadoop3-check/output/Dockerfile
GITHUB PR #3287
Optional Tests javac javadoc unit shadedjars compile
uname Linux 3376807a9d61 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 / 7c24ed4
Default Java AdoptOpenJDK-11.0.10+9
Test Results https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-3287/1/testReport/
Max. process+thread count 3753 (vs. ulimit of 30000)
modules C: hbase-common hbase-server U: .
Console output https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-3287/1/console
versions git=2.17.1 maven=3.6.3
Powered by Apache Yetus 0.12.0 https://yetus.apache.org

This message was automatically generated.

@Apache-HBase
Copy link

🎊 +1 overall

Vote Subsystem Runtime Comment
+0 🆗 reexec 3m 54s Docker mode activated.
-0 ⚠️ 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 🆗 mvndep 0m 22s Maven dependency ordering for branch
+1 💚 mvninstall 3m 53s master passed
+1 💚 compile 1m 24s master passed
+1 💚 shadedjars 8m 11s branch has no errors when building our shaded downstream artifacts.
+1 💚 javadoc 1m 0s master passed
_ Patch Compile Tests _
+0 🆗 mvndep 0m 16s Maven dependency ordering for patch
+1 💚 mvninstall 3m 41s the patch passed
+1 💚 compile 1m 25s the patch passed
+1 💚 javac 1m 25s the patch passed
+1 💚 shadedjars 8m 5s patch has no errors when building our shaded downstream artifacts.
+1 💚 javadoc 0m 57s the patch passed
_ Other Tests _
+1 💚 unit 1m 49s hbase-common in the patch passed.
+1 💚 unit 146m 57s hbase-server in the patch passed.
184m 33s
Subsystem Report/Notes
Docker ClientAPI=1.41 ServerAPI=1.41 base: https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-3287/1/artifact/yetus-jdk8-hadoop3-check/output/Dockerfile
GITHUB PR #3287
Optional Tests javac javadoc unit shadedjars compile
uname Linux e852c131c085 4.15.0-136-generic #140-Ubuntu SMP Thu Jan 28 05:20:47 UTC 2021 x86_64 x86_64 x86_64 GNU/Linux
Build tool maven
Personality dev-support/hbase-personality.sh
git revision master / 7c24ed4
Default Java AdoptOpenJDK-1.8.0_282-b08
Test Results https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-3287/1/testReport/
Max. process+thread count 3744 (vs. ulimit of 30000)
modules C: hbase-common hbase-server U: .
Console output https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-3287/1/console
versions git=2.17.1 maven=3.6.3
Powered by Apache Yetus 0.12.0 https://yetus.apache.org

This message was automatically generated.

@Apache9
Copy link
Contributor

Apache9 commented May 21, 2021

I think the goal for HBASE-23055 is to give us a way to not need to always upgrade region server first when there is a meta schema change, so I think we should try to do this less hacky.
In general, after master starts, it should check whether the meta has all the required families, if not, it should schedule a ModifyTableProcedure to change the meta schema, instead of updating the table descriptor directly, as a ModifyTableProcedure will trigger a region reopen at region server side, so we do not need to hack at region server side.
Thanks.

@virajjasani
Copy link
Contributor

I think it would be better to catch NoSuchColumnFamilyException by master and then let master schedule ModifyTableProcedure which can take care of adding missing CF. Performing this operation with Procedure should save us from any missing edge case or possible hidden race condition.

@virajjasani
Copy link
Contributor

Closing as resolved by #3417 , Thanks @pankaj72981

@virajjasani virajjasani closed this Jul 1, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants