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 2021/10/04 14:33:15 UTC
[GitHub] [hbase] wchevreuil opened a new pull request #3721: HBASE-26326 CreateTableProcedure fails when FileBasedStoreFileTracker…
wchevreuil opened a new pull request #3721:
URL: https://github.com/apache/hbase/pull/3721
… is set in global config
--
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.
To unsubscribe, e-mail: issues-unsubscribe@hbase.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [hbase] Apache9 edited a comment on pull request #3721: HBASE-26326 CreateTableProcedure fails when FileBasedStoreFileTracker…
Posted by GitBox <gi...@apache.org>.
Apache9 edited a comment on pull request #3721:
URL: https://github.com/apache/hbase/pull/3721#issuecomment-933571085
In general I do not like that we introduce a new init method but do not call it when verifying...
I think we should try to provide more things when constructing the StoreFileTracker. The current implementations are already lazy enough as we do not actually touch the storage, we just create some in memory objects...
So why not just pass in a RegionFileSystem for this case? Just like what we have done in the split/merge procedures.
```
/**
* Used at master side when splitting/merging regions, as we do not have a Store, thus no
* StoreContext at master side.
*/
public static StoreFileTracker create(Configuration conf, TableDescriptor td,
ColumnFamilyDescriptor cfd, HRegionFileSystem regionFs) {
StoreContext ctx =
StoreContext.getBuilder().withColumnFamilyDescriptor(cfd).withRegionFileSystem(regionFs)
.withFamilyStoreDirectoryPath(regionFs.getStoreDir(cfd.getNameAsString())).build();
return StoreFileTrackerFactory.create(mergeConfigurations(conf, td, cfd), true, ctx);
}
```
Thanks.
--
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.
To unsubscribe, e-mail: issues-unsubscribe@hbase.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [hbase] Apache-HBase commented on pull request #3721: HBASE-26326 CreateTableProcedure fails when FileBasedStoreFileTracker…
Posted by GitBox <gi...@apache.org>.
Apache-HBase commented on pull request #3721:
URL: https://github.com/apache/hbase/pull/3721#issuecomment-939223571
:broken_heart: **-1 overall**
| Vote | Subsystem | Runtime | Comment |
|:----:|----------:|--------:|:--------|
| +0 :ok: | reexec | 8m 1s | 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 _ |
||| _ HBASE-26067 Compile Tests _ |
| +1 :green_heart: | mvninstall | 4m 8s | HBASE-26067 passed |
| +1 :green_heart: | compile | 1m 4s | HBASE-26067 passed |
| +1 :green_heart: | shadedjars | 9m 10s | branch has no errors when building our shaded downstream artifacts. |
| +1 :green_heart: | javadoc | 0m 38s | HBASE-26067 passed |
| -0 :warning: | patch | 9m 58s | Used diff version of patch file. Binary files and potentially other changes not applied. Please rebase and squash commits if necessary. |
||| _ Patch Compile Tests _ |
| +1 :green_heart: | mvninstall | 4m 9s | the patch passed |
| +1 :green_heart: | compile | 1m 5s | the patch passed |
| +1 :green_heart: | javac | 1m 5s | the patch passed |
| +1 :green_heart: | shadedjars | 9m 8s | patch has no errors when building our shaded downstream artifacts. |
| +1 :green_heart: | javadoc | 0m 37s | the patch passed |
||| _ Other Tests _ |
| -1 :x: | unit | 223m 12s | hbase-server in the patch failed. |
| | | 263m 8s | |
| 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-3721/14/artifact/yetus-jdk8-hadoop3-check/output/Dockerfile |
| GITHUB PR | https://github.com/apache/hbase/pull/3721 |
| Optional Tests | javac javadoc unit shadedjars compile |
| uname | Linux f3740711d700 4.15.0-143-generic #147-Ubuntu SMP Wed Apr 14 16:10:11 UTC 2021 x86_64 x86_64 x86_64 GNU/Linux |
| Build tool | maven |
| Personality | dev-support/hbase-personality.sh |
| git revision | HBASE-26067 / c4d7d28911 |
| Default Java | AdoptOpenJDK-1.8.0_282-b08 |
| unit | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-3721/14/artifact/yetus-jdk8-hadoop3-check/output/patch-unit-hbase-server.txt |
| Test Results | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-3721/14/testReport/ |
| Max. process+thread count | 3336 (vs. ulimit of 30000) |
| modules | C: hbase-server U: hbase-server |
| Console output | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-3721/14/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.
--
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.
To unsubscribe, e-mail: issues-unsubscribe@hbase.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [hbase] wchevreuil merged pull request #3721: HBASE-26326 CreateTableProcedure fails when FileBasedStoreFileTracker…
Posted by GitBox <gi...@apache.org>.
wchevreuil merged pull request #3721:
URL: https://github.com/apache/hbase/pull/3721
--
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.
To unsubscribe, e-mail: issues-unsubscribe@hbase.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [hbase] Apache-HBase commented on pull request #3721: HBASE-26326 CreateTableProcedure fails when FileBasedStoreFileTracker…
Posted by GitBox <gi...@apache.org>.
Apache-HBase commented on pull request #3721:
URL: https://github.com/apache/hbase/pull/3721#issuecomment-939101380
:confetti_ball: **+1 overall**
| Vote | Subsystem | Runtime | Comment |
|:----:|----------:|--------:|:--------|
| +0 :ok: | reexec | 0m 47s | Docker mode activated. |
||| _ Prechecks _ |
| +1 :green_heart: | dupname | 0m 0s | No case conflicting files found. |
| +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. |
||| _ HBASE-26067 Compile Tests _ |
| +1 :green_heart: | mvninstall | 3m 52s | HBASE-26067 passed |
| +1 :green_heart: | compile | 3m 23s | HBASE-26067 passed |
| +1 :green_heart: | checkstyle | 1m 3s | HBASE-26067 passed |
| +1 :green_heart: | spotbugs | 2m 19s | HBASE-26067 passed |
| -0 :warning: | patch | 2m 27s | Used diff version of patch file. Binary files and potentially other changes not applied. Please rebase and squash commits if necessary. |
||| _ Patch Compile Tests _ |
| +1 :green_heart: | mvninstall | 4m 4s | the patch passed |
| +1 :green_heart: | compile | 3m 32s | the patch passed |
| +1 :green_heart: | javac | 3m 32s | the patch passed |
| -0 :warning: | checkstyle | 1m 3s | hbase-server: The patch generated 1 new + 0 unchanged - 0 fixed = 1 total (was 0) |
| +1 :green_heart: | whitespace | 0m 0s | The patch has no whitespace issues. |
| +1 :green_heart: | hadoopcheck | 19m 36s | Patch does not cause any errors with Hadoop 3.1.2 3.2.1 3.3.0. |
| +1 :green_heart: | spotbugs | 2m 28s | the patch passed |
||| _ Other Tests _ |
| +1 :green_heart: | asflicense | 0m 13s | The patch does not generate ASF License warnings. |
| | | 50m 59s | |
| 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-3721/13/artifact/yetus-general-check/output/Dockerfile |
| GITHUB PR | https://github.com/apache/hbase/pull/3721 |
| Optional Tests | dupname asflicense javac spotbugs hadoopcheck hbaseanti checkstyle compile |
| uname | Linux 79f46835fd32 4.15.0-156-generic #163-Ubuntu SMP Thu Aug 19 23:31:58 UTC 2021 x86_64 x86_64 x86_64 GNU/Linux |
| Build tool | maven |
| Personality | dev-support/hbase-personality.sh |
| git revision | HBASE-26067 / c4d7d28911 |
| Default Java | AdoptOpenJDK-1.8.0_282-b08 |
| checkstyle | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-3721/13/artifact/yetus-general-check/output/diff-checkstyle-hbase-server.txt |
| Max. process+thread count | 96 (vs. ulimit of 30000) |
| modules | C: hbase-server U: hbase-server |
| Console output | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-3721/13/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.
--
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.
To unsubscribe, e-mail: issues-unsubscribe@hbase.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [hbase] Apache-HBase commented on pull request #3721: HBASE-26326 CreateTableProcedure fails when FileBasedStoreFileTracker…
Posted by GitBox <gi...@apache.org>.
Apache-HBase commented on pull request #3721:
URL: https://github.com/apache/hbase/pull/3721#issuecomment-939194598
:confetti_ball: **+1 overall**
| Vote | Subsystem | Runtime | Comment |
|:----:|----------:|--------:|:--------|
| +0 :ok: | reexec | 1m 2s | Docker mode activated. |
||| _ Prechecks _ |
| +1 :green_heart: | dupname | 0m 0s | No case conflicting files found. |
| +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. |
||| _ HBASE-26067 Compile Tests _ |
| +1 :green_heart: | mvninstall | 4m 24s | HBASE-26067 passed |
| +1 :green_heart: | compile | 3m 23s | HBASE-26067 passed |
| +1 :green_heart: | checkstyle | 1m 12s | HBASE-26067 passed |
| +1 :green_heart: | spotbugs | 2m 16s | HBASE-26067 passed |
| -0 :warning: | patch | 2m 24s | Used diff version of patch file. Binary files and potentially other changes not applied. Please rebase and squash commits if necessary. |
||| _ Patch Compile Tests _ |
| +1 :green_heart: | mvninstall | 4m 3s | the patch passed |
| +1 :green_heart: | compile | 3m 23s | the patch passed |
| +1 :green_heart: | javac | 3m 23s | the patch passed |
| +1 :green_heart: | checkstyle | 1m 8s | the patch passed |
| +1 :green_heart: | whitespace | 0m 0s | The patch has no whitespace issues. |
| +1 :green_heart: | hadoopcheck | 20m 28s | Patch does not cause any errors with Hadoop 3.1.2 3.2.1 3.3.0. |
| +1 :green_heart: | spotbugs | 2m 22s | the patch passed |
||| _ Other Tests _ |
| +1 :green_heart: | asflicense | 0m 13s | The patch does not generate ASF License warnings. |
| | | 52m 20s | |
| 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-3721/14/artifact/yetus-general-check/output/Dockerfile |
| GITHUB PR | https://github.com/apache/hbase/pull/3721 |
| Optional Tests | dupname asflicense javac spotbugs hadoopcheck hbaseanti checkstyle compile |
| uname | Linux 509d23af2a3b 4.15.0-153-generic #160-Ubuntu SMP Thu Jul 29 06:54:29 UTC 2021 x86_64 x86_64 x86_64 GNU/Linux |
| Build tool | maven |
| Personality | dev-support/hbase-personality.sh |
| git revision | HBASE-26067 / c4d7d28911 |
| Default Java | AdoptOpenJDK-1.8.0_282-b08 |
| Max. process+thread count | 86 (vs. ulimit of 30000) |
| modules | C: hbase-server U: hbase-server |
| Console output | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-3721/14/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.
--
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.
To unsubscribe, e-mail: issues-unsubscribe@hbase.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [hbase] wchevreuil commented on a change in pull request #3721: HBASE-26326 CreateTableProcedure fails when FileBasedStoreFileTracker…
Posted by GitBox <gi...@apache.org>.
wchevreuil commented on a change in pull request #3721:
URL: https://github.com/apache/hbase/pull/3721#discussion_r726129081
##########
File path: hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/storefiletracker/MigrationStoreFileTracker.java
##########
@@ -89,14 +90,16 @@ void set(List<StoreFileInfo> files) {
}
@Override
- public void persistConfiguration(TableDescriptorBuilder builder) {
- super.persistConfiguration(builder);
- if (StringUtils.isEmpty(builder.getValue(SRC_IMPL))) {
+ public TableDescriptor updateWithTrackerConfigs(TableDescriptor descriptor) {
Review comment:
Right. Is it not worth having it here, case we need to call this from elsewhere, other than table creation? I was planning to extend MigrationStoreFileTracker for the snapshot issue I'm working on HBASE-26328, so that I have a snapshot sft as source, and whatever is the SFT impl defined as target. Ain't sure now if this method would be required from there, though.
--
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.
To unsubscribe, e-mail: issues-unsubscribe@hbase.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [hbase] Apache-HBase commented on pull request #3721: HBASE-26326 CreateTableProcedure fails when FileBasedStoreFileTracker…
Posted by GitBox <gi...@apache.org>.
Apache-HBase commented on pull request #3721:
URL: https://github.com/apache/hbase/pull/3721#issuecomment-939183808
:broken_heart: **-1 overall**
| Vote | Subsystem | Runtime | Comment |
|:----:|----------:|--------:|:--------|
| +0 :ok: | reexec | 1m 21s | 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 _ |
||| _ HBASE-26067 Compile Tests _ |
| +1 :green_heart: | mvninstall | 4m 7s | HBASE-26067 passed |
| +1 :green_heart: | compile | 1m 5s | HBASE-26067 passed |
| +1 :green_heart: | shadedjars | 9m 11s | branch has no errors when building our shaded downstream artifacts. |
| +1 :green_heart: | javadoc | 0m 36s | HBASE-26067 passed |
| -0 :warning: | patch | 9m 57s | Used diff version of patch file. Binary files and potentially other changes not applied. Please rebase and squash commits if necessary. |
||| _ Patch Compile Tests _ |
| +1 :green_heart: | mvninstall | 4m 12s | the patch passed |
| +1 :green_heart: | compile | 1m 6s | the patch passed |
| +1 :green_heart: | javac | 1m 6s | the patch passed |
| +1 :green_heart: | shadedjars | 9m 4s | patch has no errors when building our shaded downstream artifacts. |
| +1 :green_heart: | javadoc | 0m 37s | the patch passed |
||| _ Other Tests _ |
| -1 :x: | unit | 221m 42s | hbase-server in the patch failed. |
| | | 255m 0s | |
| 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-3721/13/artifact/yetus-jdk8-hadoop3-check/output/Dockerfile |
| GITHUB PR | https://github.com/apache/hbase/pull/3721 |
| Optional Tests | javac javadoc unit shadedjars compile |
| uname | Linux 0c1e9bb9fdf3 4.15.0-143-generic #147-Ubuntu SMP Wed Apr 14 16:10:11 UTC 2021 x86_64 x86_64 x86_64 GNU/Linux |
| Build tool | maven |
| Personality | dev-support/hbase-personality.sh |
| git revision | HBASE-26067 / c4d7d28911 |
| Default Java | AdoptOpenJDK-1.8.0_282-b08 |
| unit | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-3721/13/artifact/yetus-jdk8-hadoop3-check/output/patch-unit-hbase-server.txt |
| Test Results | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-3721/13/testReport/ |
| Max. process+thread count | 2964 (vs. ulimit of 30000) |
| modules | C: hbase-server U: hbase-server |
| Console output | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-3721/13/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.
--
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.
To unsubscribe, e-mail: issues-unsubscribe@hbase.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [hbase] wchevreuil edited a comment on pull request #3721: HBASE-26326 CreateTableProcedure fails when FileBasedStoreFileTracker…
Posted by GitBox <gi...@apache.org>.
wchevreuil edited a comment on pull request #3721:
URL: https://github.com/apache/hbase/pull/3721#issuecomment-937818470
> The TestStoreFileTracker.testHasPersistConfiguration method is used to make sure that all the implementation classes have the correct static method.
My problem with this approach is that we then don't have an intuitive interface that clearly states what should be implemented, requiring this extra hack to enforce its rules, at test execution time only. I would rather have a more strict interface that it's easier for developer to understand what is required to be implemented (the original approach), or, since this seems to be an issue exclusively for the FILE SFT impl, let it deal with the config mode on its own (the extra check for missing info in the passed StoreContext implemented on last commit). WDYT?
> For the naming change, updateDescriptor seems too general, what is the reason for changing the method name? I do not mean we can not change it, just want to know the reason.
While going through this, I thought the `persistConfiguration` confusing. Even though I was the author of this method on few commits back, now whenever I was reading it, I was misled to think it was persisting the configuration somewhere, when in reality it was just updating the passed descriptor builder with additional configs needed by individual SFT impl. As it seems `updateDescriptor` still sounds confusing, let me try something else. How about `updateWithTrackerConfigs`?
--
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.
To unsubscribe, e-mail: issues-unsubscribe@hbase.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [hbase] Apache9 commented on pull request #3721: HBASE-26326 CreateTableProcedure fails when FileBasedStoreFileTracker…
Posted by GitBox <gi...@apache.org>.
Apache9 commented on pull request #3721:
URL: https://github.com/apache/hbase/pull/3721#issuecomment-937804090
This is an approach to force a static method in all the StoreFileTracker implementation classes for persisting the configurations.
https://github.com/Apache9/hbase/commit/da63c02057c49a70baafbe6ae533d506f7469a08
The TestStoreFileTracker.testHasPersistConfiguration method is used to make sure that all the implementation classes have the correct static method.
@wchevreuil PTAL. I think this could make the normal code path cleaner.
For the naming change, updateDescriptor seems too general, what is the reason for changing the method name? I do not mean we can not change it, just want to know the reason.
Thanks.
--
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.
To unsubscribe, e-mail: issues-unsubscribe@hbase.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [hbase] wchevreuil commented on a change in pull request #3721: HBASE-26326 CreateTableProcedure fails when FileBasedStoreFileTracker…
Posted by GitBox <gi...@apache.org>.
wchevreuil commented on a change in pull request #3721:
URL: https://github.com/apache/hbase/pull/3721#discussion_r727866389
##########
File path: hbase-server/src/test/java/org/apache/hadoop/hbase/master/procedure/TestCreateTableProcedure.java
##########
@@ -105,6 +106,21 @@ public void testCreateWithTrackImpl() throws Exception {
assertEquals(trackerName, htd.getValue(TRACKER_IMPL));
}
+ @Test
+ public void testCreateWithFileBasedStoreTrackerImpl() throws Exception {
+ ProcedureExecutor<MasterProcedureEnv> procExec = getMasterProcedureExecutor();
+ procExec.getEnvironment().getMasterConfiguration().set(StoreFileTrackerFactory.TRACKER_IMPL,
+ StoreFileTrackerFactory.Trackers.FILE.name());
Review comment:
Yes, just checked now and found out that this relies on a static HBaseTestingUtil variable declared in the parent TestTableDDLProcedureBase class, so any concurrent sub-classes instances of that could end up picking that config.
I don't think this would be a problem for other tests, though, since none of those are validating store file tracking work itself, and whatever is being tested elsewhere shouldn't break because of this setting (if it does break some other test, then it's probably a regression we are introducing here).
The only potential problem is if both this testCreateWithFileBasedStoreTrackerImpl and testCreateWithTrackImpl test methods run concurrently, but I don't think that happens for methods within the same test class.
--
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.
To unsubscribe, e-mail: issues-unsubscribe@hbase.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [hbase] Apache-HBase commented on pull request #3721: HBASE-26326 CreateTableProcedure fails when FileBasedStoreFileTracker…
Posted by GitBox <gi...@apache.org>.
Apache-HBase commented on pull request #3721:
URL: https://github.com/apache/hbase/pull/3721#issuecomment-933758872
:broken_heart: **-1 overall**
| Vote | Subsystem | Runtime | Comment |
|:----:|----------:|--------:|:--------|
| +0 :ok: | reexec | 6m 5s | 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 _ |
||| _ HBASE-26067 Compile Tests _ |
| +1 :green_heart: | mvninstall | 5m 3s | HBASE-26067 passed |
| +1 :green_heart: | compile | 1m 20s | HBASE-26067 passed |
| +1 :green_heart: | shadedjars | 9m 8s | branch has no errors when building our shaded downstream artifacts. |
| +1 :green_heart: | javadoc | 0m 45s | HBASE-26067 passed |
||| _ Patch Compile Tests _ |
| +1 :green_heart: | mvninstall | 4m 49s | the patch passed |
| +1 :green_heart: | compile | 1m 22s | the patch passed |
| +1 :green_heart: | javac | 1m 22s | the patch passed |
| +1 :green_heart: | shadedjars | 9m 6s | patch has no errors when building our shaded downstream artifacts. |
| +1 :green_heart: | javadoc | 0m 42s | the patch passed |
||| _ Other Tests _ |
| -1 :x: | unit | 215m 21s | hbase-server in the patch failed. |
| | | 255m 42s | |
| 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-3721/1/artifact/yetus-jdk11-hadoop3-check/output/Dockerfile |
| GITHUB PR | https://github.com/apache/hbase/pull/3721 |
| Optional Tests | javac javadoc unit shadedjars compile |
| uname | Linux c1d09c78fdaf 4.15.0-142-generic #146-Ubuntu SMP Tue Apr 13 01:11:19 UTC 2021 x86_64 x86_64 x86_64 GNU/Linux |
| Build tool | maven |
| Personality | dev-support/hbase-personality.sh |
| git revision | HBASE-26067 / ab03ee1e97 |
| Default Java | AdoptOpenJDK-11.0.10+9 |
| unit | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-3721/1/artifact/yetus-jdk11-hadoop3-check/output/patch-unit-hbase-server.txt |
| Test Results | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-3721/1/testReport/ |
| Max. process+thread count | 3213 (vs. ulimit of 30000) |
| modules | C: hbase-server U: hbase-server |
| Console output | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-3721/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.
--
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.
To unsubscribe, e-mail: issues-unsubscribe@hbase.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [hbase] Apache-HBase commented on pull request #3721: HBASE-26326 CreateTableProcedure fails when FileBasedStoreFileTracker…
Posted by GitBox <gi...@apache.org>.
Apache-HBase commented on pull request #3721:
URL: https://github.com/apache/hbase/pull/3721#issuecomment-937934737
:confetti_ball: **+1 overall**
| Vote | Subsystem | Runtime | Comment |
|:----:|----------:|--------:|:--------|
| +0 :ok: | reexec | 0m 25s | Docker mode activated. |
||| _ Prechecks _ |
| +1 :green_heart: | dupname | 0m 0s | No case conflicting files found. |
| +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. |
||| _ HBASE-26067 Compile Tests _ |
| +1 :green_heart: | mvninstall | 3m 41s | HBASE-26067 passed |
| +1 :green_heart: | compile | 3m 14s | HBASE-26067 passed |
| +1 :green_heart: | checkstyle | 1m 3s | HBASE-26067 passed |
| +1 :green_heart: | spotbugs | 2m 6s | HBASE-26067 passed |
| -0 :warning: | patch | 2m 14s | Used diff version of patch file. Binary files and potentially other changes not applied. Please rebase and squash commits if necessary. |
||| _ Patch Compile Tests _ |
| +1 :green_heart: | mvninstall | 3m 40s | the patch passed |
| +1 :green_heart: | compile | 3m 16s | the patch passed |
| +1 :green_heart: | javac | 3m 16s | the patch passed |
| -0 :warning: | checkstyle | 1m 2s | hbase-server: The patch generated 1 new + 0 unchanged - 0 fixed = 1 total (was 0) |
| +1 :green_heart: | whitespace | 0m 0s | The patch has no whitespace issues. |
| +1 :green_heart: | hadoopcheck | 18m 43s | Patch does not cause any errors with Hadoop 3.1.2 3.2.1 3.3.0. |
| +1 :green_heart: | spotbugs | 2m 15s | the patch passed |
||| _ Other Tests _ |
| +1 :green_heart: | asflicense | 0m 13s | The patch does not generate ASF License warnings. |
| | | 47m 35s | |
| 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-3721/10/artifact/yetus-general-check/output/Dockerfile |
| GITHUB PR | https://github.com/apache/hbase/pull/3721 |
| Optional Tests | dupname asflicense javac spotbugs hadoopcheck hbaseanti checkstyle compile |
| uname | Linux 2ec73e1b99a2 4.15.0-112-generic #113-Ubuntu SMP Thu Jul 9 23:41:39 UTC 2020 x86_64 x86_64 x86_64 GNU/Linux |
| Build tool | maven |
| Personality | dev-support/hbase-personality.sh |
| git revision | HBASE-26067 / c4d7d28911 |
| Default Java | AdoptOpenJDK-1.8.0_282-b08 |
| checkstyle | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-3721/10/artifact/yetus-general-check/output/diff-checkstyle-hbase-server.txt |
| Max. process+thread count | 96 (vs. ulimit of 30000) |
| modules | C: hbase-server U: hbase-server |
| Console output | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-3721/10/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.
--
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.
To unsubscribe, e-mail: issues-unsubscribe@hbase.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [hbase] Apache-HBase commented on pull request #3721: HBASE-26326 CreateTableProcedure fails when FileBasedStoreFileTracker…
Posted by GitBox <gi...@apache.org>.
Apache-HBase commented on pull request #3721:
URL: https://github.com/apache/hbase/pull/3721#issuecomment-941532157
--
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.
To unsubscribe, e-mail: issues-unsubscribe@hbase.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [hbase] Apache-HBase commented on pull request #3721: HBASE-26326 CreateTableProcedure fails when FileBasedStoreFileTracker…
Posted by GitBox <gi...@apache.org>.
Apache-HBase commented on pull request #3721:
URL: https://github.com/apache/hbase/pull/3721#issuecomment-937635219
:confetti_ball: **+1 overall**
| Vote | Subsystem | Runtime | Comment |
|:----:|----------:|--------:|:--------|
| +0 :ok: | reexec | 0m 30s | Docker mode activated. |
||| _ Prechecks _ |
| +1 :green_heart: | dupname | 0m 0s | No case conflicting files found. |
| +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. |
||| _ HBASE-26067 Compile Tests _ |
| +1 :green_heart: | mvninstall | 4m 7s | HBASE-26067 passed |
| +1 :green_heart: | compile | 3m 26s | HBASE-26067 passed |
| +1 :green_heart: | checkstyle | 1m 0s | HBASE-26067 passed |
| +1 :green_heart: | spotbugs | 2m 2s | HBASE-26067 passed |
| -0 :warning: | patch | 2m 11s | Used diff version of patch file. Binary files and potentially other changes not applied. Please rebase and squash commits if necessary. |
||| _ Patch Compile Tests _ |
| +1 :green_heart: | mvninstall | 3m 39s | the patch passed |
| +1 :green_heart: | compile | 3m 9s | the patch passed |
| +1 :green_heart: | javac | 3m 9s | the patch passed |
| -0 :warning: | checkstyle | 1m 3s | hbase-server: The patch generated 1 new + 0 unchanged - 0 fixed = 1 total (was 0) |
| +1 :green_heart: | whitespace | 0m 0s | The patch has no whitespace issues. |
| +1 :green_heart: | hadoopcheck | 18m 17s | Patch does not cause any errors with Hadoop 3.1.2 3.2.1 3.3.0. |
| +1 :green_heart: | spotbugs | 2m 14s | the patch passed |
||| _ Other Tests _ |
| +1 :green_heart: | asflicense | 0m 13s | The patch does not generate ASF License warnings. |
| | | 47m 40s | |
| 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-3721/8/artifact/yetus-general-check/output/Dockerfile |
| GITHUB PR | https://github.com/apache/hbase/pull/3721 |
| Optional Tests | dupname asflicense javac spotbugs hadoopcheck hbaseanti checkstyle compile |
| uname | Linux cece6ab30775 4.15.0-156-generic #163-Ubuntu SMP Thu Aug 19 23:31:58 UTC 2021 x86_64 x86_64 x86_64 GNU/Linux |
| Build tool | maven |
| Personality | dev-support/hbase-personality.sh |
| git revision | HBASE-26067 / c4d7d28911 |
| Default Java | AdoptOpenJDK-1.8.0_282-b08 |
| checkstyle | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-3721/8/artifact/yetus-general-check/output/diff-checkstyle-hbase-server.txt |
| Max. process+thread count | 96 (vs. ulimit of 30000) |
| modules | C: hbase-server U: hbase-server |
| Console output | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-3721/8/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.
--
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.
To unsubscribe, e-mail: issues-unsubscribe@hbase.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [hbase] joshelser commented on a change in pull request #3721: HBASE-26326 CreateTableProcedure fails when FileBasedStoreFileTracker…
Posted by GitBox <gi...@apache.org>.
joshelser commented on a change in pull request #3721:
URL: https://github.com/apache/hbase/pull/3721#discussion_r727605083
##########
File path: hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/storefiletracker/StoreFileTracker.java
##########
@@ -75,13 +76,13 @@ void replace(Collection<StoreFileInfo> compactedFiles, Collection<StoreFileInfo>
StoreFileWriter createWriter(CreateStoreFileWriterParams params) throws IOException;
/**
- * Saves StoreFileTracker implementations specific configurations into the table descriptors.
+ * Adds StoreFileTracker implementations specific configurations into the table descriptor.
* <p/>
* This is used to avoid accidentally data loss when changing the cluster level store file tracker
* implementation, and also possible misconfiguration between master and region servers.
* <p/>
* See HBASE-26246 for more details.
* @param builder The table descriptor builder for the given table.
*/
- void persistConfiguration(TableDescriptorBuilder builder);
+ TableDescriptor updateWithTrackerConfigs(TableDescriptorBuilder builder);
Review comment:
Maybe `buildWithTrackerConfigs` if you're returning the `TableDescriptor` instead of the `TableDescriptorBuilder`. IMO, i'd just return the Builder back and let the caller `build()` when they're ready.
##########
File path: hbase-server/src/test/java/org/apache/hadoop/hbase/master/procedure/TestCreateTableProcedure.java
##########
@@ -105,6 +106,21 @@ public void testCreateWithTrackImpl() throws Exception {
assertEquals(trackerName, htd.getValue(TRACKER_IMPL));
}
+ @Test
+ public void testCreateWithFileBasedStoreTrackerImpl() throws Exception {
+ ProcedureExecutor<MasterProcedureEnv> procExec = getMasterProcedureExecutor();
+ procExec.getEnvironment().getMasterConfiguration().set(StoreFileTrackerFactory.TRACKER_IMPL,
+ StoreFileTrackerFactory.Trackers.FILE.name());
Review comment:
Does this need to be unset for other test methods in this class? Or, if tests are executed in parallel, maybe it would affects the other methods in this class? We set `forkCount` in surefire-plugin, but we don't set `parallel` (which I guess means we don't do parallel tests?). Genuinely not sure :)
--
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.
To unsubscribe, e-mail: issues-unsubscribe@hbase.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [hbase] Apache-HBase commented on pull request #3721: HBASE-26326 CreateTableProcedure fails when FileBasedStoreFileTracker…
Posted by GitBox <gi...@apache.org>.
Apache-HBase commented on pull request #3721:
URL: https://github.com/apache/hbase/pull/3721#issuecomment-938714417
:confetti_ball: **+1 overall**
| Vote | Subsystem | Runtime | Comment |
|:----:|----------:|--------:|:--------|
| +0 :ok: | reexec | 1m 21s | 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 _ |
||| _ HBASE-26067 Compile Tests _ |
| +1 :green_heart: | mvninstall | 4m 15s | HBASE-26067 passed |
| +1 :green_heart: | compile | 1m 5s | HBASE-26067 passed |
| +1 :green_heart: | shadedjars | 9m 10s | branch has no errors when building our shaded downstream artifacts. |
| +1 :green_heart: | javadoc | 0m 37s | HBASE-26067 passed |
| -0 :warning: | patch | 9m 58s | Used diff version of patch file. Binary files and potentially other changes not applied. Please rebase and squash commits if necessary. |
||| _ Patch Compile Tests _ |
| +1 :green_heart: | mvninstall | 4m 6s | the patch passed |
| +1 :green_heart: | compile | 1m 4s | the patch passed |
| +1 :green_heart: | javac | 1m 4s | the patch passed |
| +1 :green_heart: | shadedjars | 9m 10s | patch has no errors when building our shaded downstream artifacts. |
| +1 :green_heart: | javadoc | 0m 37s | the patch passed |
||| _ Other Tests _ |
| +1 :green_heart: | unit | 222m 17s | hbase-server in the patch passed. |
| | | 255m 42s | |
| 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-3721/11/artifact/yetus-jdk8-hadoop3-check/output/Dockerfile |
| GITHUB PR | https://github.com/apache/hbase/pull/3721 |
| Optional Tests | javac javadoc unit shadedjars compile |
| uname | Linux d0a2f3193300 4.15.0-143-generic #147-Ubuntu SMP Wed Apr 14 16:10:11 UTC 2021 x86_64 x86_64 x86_64 GNU/Linux |
| Build tool | maven |
| Personality | dev-support/hbase-personality.sh |
| git revision | HBASE-26067 / c4d7d28911 |
| Default Java | AdoptOpenJDK-1.8.0_282-b08 |
| Test Results | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-3721/11/testReport/ |
| Max. process+thread count | 3297 (vs. ulimit of 30000) |
| modules | C: hbase-server U: hbase-server |
| Console output | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-3721/11/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.
--
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.
To unsubscribe, e-mail: issues-unsubscribe@hbase.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [hbase] Apache-HBase commented on pull request #3721: HBASE-26326 CreateTableProcedure fails when FileBasedStoreFileTracker…
Posted by GitBox <gi...@apache.org>.
Apache-HBase commented on pull request #3721:
URL: https://github.com/apache/hbase/pull/3721#issuecomment-939066233
:broken_heart: **-1 overall**
| Vote | Subsystem | Runtime | Comment |
|:----:|----------:|--------:|:--------|
| +0 :ok: | reexec | 1m 18s | 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 _ |
||| _ HBASE-26067 Compile Tests _ |
| +1 :green_heart: | mvninstall | 4m 10s | HBASE-26067 passed |
| +1 :green_heart: | compile | 1m 4s | HBASE-26067 passed |
| +1 :green_heart: | shadedjars | 9m 4s | branch has no errors when building our shaded downstream artifacts. |
| +1 :green_heart: | javadoc | 0m 35s | HBASE-26067 passed |
| -0 :warning: | patch | 9m 49s | Used diff version of patch file. Binary files and potentially other changes not applied. Please rebase and squash commits if necessary. |
||| _ Patch Compile Tests _ |
| +1 :green_heart: | mvninstall | 4m 11s | the patch passed |
| +1 :green_heart: | compile | 1m 4s | the patch passed |
| +1 :green_heart: | javac | 1m 4s | the patch passed |
| +1 :green_heart: | shadedjars | 9m 11s | patch has no errors when building our shaded downstream artifacts. |
| +1 :green_heart: | javadoc | 0m 35s | the patch passed |
||| _ Other Tests _ |
| -1 :x: | unit | 222m 36s | hbase-server in the patch failed. |
| | | 255m 49s | |
| 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-3721/12/artifact/yetus-jdk8-hadoop3-check/output/Dockerfile |
| GITHUB PR | https://github.com/apache/hbase/pull/3721 |
| Optional Tests | javac javadoc unit shadedjars compile |
| uname | Linux 39776a1888eb 4.15.0-143-generic #147-Ubuntu SMP Wed Apr 14 16:10:11 UTC 2021 x86_64 x86_64 x86_64 GNU/Linux |
| Build tool | maven |
| Personality | dev-support/hbase-personality.sh |
| git revision | HBASE-26067 / c4d7d28911 |
| Default Java | AdoptOpenJDK-1.8.0_282-b08 |
| unit | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-3721/12/artifact/yetus-jdk8-hadoop3-check/output/patch-unit-hbase-server.txt |
| Test Results | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-3721/12/testReport/ |
| Max. process+thread count | 3362 (vs. ulimit of 30000) |
| modules | C: hbase-server U: hbase-server |
| Console output | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-3721/12/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.
--
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.
To unsubscribe, e-mail: issues-unsubscribe@hbase.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [hbase] Apache9 commented on pull request #3721: HBASE-26326 CreateTableProcedure fails when FileBasedStoreFileTracker…
Posted by GitBox <gi...@apache.org>.
Apache9 commented on pull request #3721:
URL: https://github.com/apache/hbase/pull/3721#issuecomment-934436283
> We don't have store info at CreateTable procedure time, we don't even have the regions yet by then, so how can we create a RegionFileSystem?
Yes, this is a problem. In the first place I thought maybe we could just create a default region and then create a region file system, but it seems to tricky and a bit ugly. So we should let the implementation class to know this. A possible way is to pass in a null StoreContext, and the implementation class should check whether the StoreContext is null, if so, then it should construct itself in the 'persist store tracker configurations' mode.
Or another possible way is to force every store file tracker implementation has a static method, for persisting the store file tracker configurations. We could have separated logic in this method, so normal code path does not need to deal with null StoreContext.
I think the latter one will be cleaner but not sure if we can force the implementation class has a static method at compile time. But at least, we could introduce a UT for checking this.
WDYT? Thanks.
--
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.
To unsubscribe, e-mail: issues-unsubscribe@hbase.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [hbase] wchevreuil commented on pull request #3721: HBASE-26326 CreateTableProcedure fails when FileBasedStoreFileTracker…
Posted by GitBox <gi...@apache.org>.
wchevreuil commented on pull request #3721:
URL: https://github.com/apache/hbase/pull/3721#issuecomment-937158799
I'm sorry @Apache9 , but why did you force push all these commits into the branch? I believe it had no conflicts with HBASE-26067 before.
--
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.
To unsubscribe, e-mail: issues-unsubscribe@hbase.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [hbase] Apache9 commented on pull request #3721: HBASE-26326 CreateTableProcedure fails when FileBasedStoreFileTracker…
Posted by GitBox <gi...@apache.org>.
Apache9 commented on pull request #3721:
URL: https://github.com/apache/hbase/pull/3721#issuecomment-937331616
I just rebased HBASE-26067 against the newest master branch, and seems GitHub can not identify this type of rebase so it will show all the commits in the PR. You can do a cherry-pick to the newest HBASE-26067 and then do a force push to your branch.
Sorry for the inconvenience. In the future I will shout before doing the rebase to let others know.
--
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.
To unsubscribe, e-mail: issues-unsubscribe@hbase.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [hbase] wchevreuil commented on a change in pull request #3721: HBASE-26326 CreateTableProcedure fails when FileBasedStoreFileTracker…
Posted by GitBox <gi...@apache.org>.
wchevreuil commented on a change in pull request #3721:
URL: https://github.com/apache/hbase/pull/3721#discussion_r724919715
##########
File path: hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/storefiletracker/StoreFileTrackerFactory.java
##########
@@ -100,13 +102,19 @@ static String getStoreFileTrackerName(Class<? extends StoreFileTracker> clazz) {
}
}
- public static StoreFileTracker create(Configuration conf, boolean isPrimaryReplica,
+ private static StoreFileTracker getInstance(Configuration conf, boolean isPrimaryReplica,
StoreContext ctx) {
Class<? extends StoreFileTracker> tracker = getTrackerClass(conf);
LOG.info("instantiating StoreFileTracker impl {}", tracker.getName());
return ReflectionUtils.newInstance(tracker, conf, isPrimaryReplica, ctx);
}
+ public static StoreFileTracker create(Configuration conf, boolean isPrimaryReplica,
Review comment:
Yes, this was relevant for the original solution only, when we had an init() method. let me revert this.
--
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.
To unsubscribe, e-mail: issues-unsubscribe@hbase.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [hbase] wchevreuil commented on a change in pull request #3721: HBASE-26326 CreateTableProcedure fails when FileBasedStoreFileTracker…
Posted by GitBox <gi...@apache.org>.
wchevreuil commented on a change in pull request #3721:
URL: https://github.com/apache/hbase/pull/3721#discussion_r727458552
##########
File path: hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/storefiletracker/MigrationStoreFileTracker.java
##########
@@ -89,14 +90,16 @@ void set(List<StoreFileInfo> files) {
}
@Override
- public void persistConfiguration(TableDescriptorBuilder builder) {
- super.persistConfiguration(builder);
- if (StringUtils.isEmpty(builder.getValue(SRC_IMPL))) {
+ public TableDescriptor updateWithTrackerConfigs(TableDescriptor descriptor) {
Review comment:
Done.
--
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.
To unsubscribe, e-mail: issues-unsubscribe@hbase.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [hbase] wchevreuil commented on a change in pull request #3721: HBASE-26326 CreateTableProcedure fails when FileBasedStoreFileTracker…
Posted by GitBox <gi...@apache.org>.
wchevreuil commented on a change in pull request #3721:
URL: https://github.com/apache/hbase/pull/3721#discussion_r724909151
##########
File path: hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/storefiletracker/FileBasedStoreFileTracker.java
##########
@@ -50,13 +50,15 @@
@InterfaceAudience.Private
class FileBasedStoreFileTracker extends StoreFileTrackerBase {
- private final StoreFileListFile backedFile;
+ private StoreFileListFile backedFile;
Review comment:
If we leave it final, it gives a compile error because of the if check in the constructor.
--
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.
To unsubscribe, e-mail: issues-unsubscribe@hbase.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [hbase] Apache9 commented on a change in pull request #3721: HBASE-26326 CreateTableProcedure fails when FileBasedStoreFileTracker…
Posted by GitBox <gi...@apache.org>.
Apache9 commented on a change in pull request #3721:
URL: https://github.com/apache/hbase/pull/3721#discussion_r728004188
##########
File path: hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/storefiletracker/StoreFileTracker.java
##########
@@ -75,13 +76,13 @@ void replace(Collection<StoreFileInfo> compactedFiles, Collection<StoreFileInfo>
StoreFileWriter createWriter(CreateStoreFileWriterParams params) throws IOException;
/**
- * Saves StoreFileTracker implementations specific configurations into the table descriptors.
+ * Adds StoreFileTracker implementations specific configurations into the table descriptor.
* <p/>
* This is used to avoid accidentally data loss when changing the cluster level store file tracker
* implementation, and also possible misconfiguration between master and region servers.
* <p/>
* See HBASE-26246 for more details.
* @param builder The table descriptor builder for the given table.
*/
- void persistConfiguration(TableDescriptorBuilder builder);
+ TableDescriptor updateWithTrackerConfigs(TableDescriptorBuilder builder);
Review comment:
Just return void is enough. The upper layer could build it after calling this method.
--
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.
To unsubscribe, e-mail: issues-unsubscribe@hbase.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [hbase] Apache-HBase commented on pull request #3721: HBASE-26326 CreateTableProcedure fails when FileBasedStoreFileTracker…
Posted by GitBox <gi...@apache.org>.
Apache-HBase commented on pull request #3721:
URL: https://github.com/apache/hbase/pull/3721#issuecomment-937730367
:broken_heart: **-1 overall**
| Vote | Subsystem | Runtime | Comment |
|:----:|----------:|--------:|:--------|
| +0 :ok: | reexec | 0m 25s | 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 _ |
||| _ HBASE-26067 Compile Tests _ |
| +1 :green_heart: | mvninstall | 3m 57s | HBASE-26067 passed |
| +1 :green_heart: | compile | 1m 1s | HBASE-26067 passed |
| +1 :green_heart: | shadedjars | 8m 15s | branch has no errors when building our shaded downstream artifacts. |
| +1 :green_heart: | javadoc | 0m 39s | HBASE-26067 passed |
| -0 :warning: | patch | 9m 6s | Used diff version of patch file. Binary files and potentially other changes not applied. Please rebase and squash commits if necessary. |
||| _ Patch Compile Tests _ |
| +1 :green_heart: | mvninstall | 3m 45s | the patch passed |
| +1 :green_heart: | compile | 1m 0s | the patch passed |
| +1 :green_heart: | javac | 1m 0s | the patch passed |
| +1 :green_heart: | shadedjars | 8m 20s | patch has no errors when building our shaded downstream artifacts. |
| +1 :green_heart: | javadoc | 0m 37s | the patch passed |
||| _ Other Tests _ |
| -1 :x: | unit | 154m 8s | hbase-server in the patch failed. |
| | | 184m 28s | |
| 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-3721/8/artifact/yetus-jdk8-hadoop3-check/output/Dockerfile |
| GITHUB PR | https://github.com/apache/hbase/pull/3721 |
| Optional Tests | javac javadoc unit shadedjars compile |
| uname | Linux 4252a5aec8c6 4.15.0-112-generic #113-Ubuntu SMP Thu Jul 9 23:41:39 UTC 2020 x86_64 x86_64 x86_64 GNU/Linux |
| Build tool | maven |
| Personality | dev-support/hbase-personality.sh |
| git revision | HBASE-26067 / c4d7d28911 |
| Default Java | AdoptOpenJDK-1.8.0_282-b08 |
| unit | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-3721/8/artifact/yetus-jdk8-hadoop3-check/output/patch-unit-hbase-server.txt |
| Test Results | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-3721/8/testReport/ |
| Max. process+thread count | 3762 (vs. ulimit of 30000) |
| modules | C: hbase-server U: hbase-server |
| Console output | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-3721/8/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.
--
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.
To unsubscribe, e-mail: issues-unsubscribe@hbase.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [hbase] Apache-HBase commented on pull request #3721: HBASE-26326 CreateTableProcedure fails when FileBasedStoreFileTracker…
Posted by GitBox <gi...@apache.org>.
Apache-HBase commented on pull request #3721:
URL: https://github.com/apache/hbase/pull/3721#issuecomment-937729030
:confetti_ball: **+1 overall**
| Vote | Subsystem | Runtime | Comment |
|:----:|----------:|--------:|:--------|
| +0 :ok: | reexec | 0m 27s | 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 _ |
||| _ HBASE-26067 Compile Tests _ |
| +1 :green_heart: | mvninstall | 4m 59s | HBASE-26067 passed |
| +1 :green_heart: | compile | 1m 21s | HBASE-26067 passed |
| +1 :green_heart: | shadedjars | 9m 6s | branch has no errors when building our shaded downstream artifacts. |
| +1 :green_heart: | javadoc | 0m 48s | HBASE-26067 passed |
| -0 :warning: | patch | 10m 6s | Used diff version of patch file. Binary files and potentially other changes not applied. Please rebase and squash commits if necessary. |
||| _ Patch Compile Tests _ |
| +1 :green_heart: | mvninstall | 4m 32s | the patch passed |
| +1 :green_heart: | compile | 1m 18s | the patch passed |
| +1 :green_heart: | javac | 1m 18s | the patch passed |
| +1 :green_heart: | shadedjars | 8m 50s | patch has no errors when building our shaded downstream artifacts. |
| +1 :green_heart: | javadoc | 0m 42s | the patch passed |
||| _ Other Tests _ |
| +1 :green_heart: | unit | 148m 41s | hbase-server in the patch passed. |
| | | 182m 49s | |
| 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-3721/8/artifact/yetus-jdk11-hadoop3-check/output/Dockerfile |
| GITHUB PR | https://github.com/apache/hbase/pull/3721 |
| Optional Tests | javac javadoc unit shadedjars compile |
| uname | Linux 24a43b82013b 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 | HBASE-26067 / c4d7d28911 |
| Default Java | AdoptOpenJDK-11.0.10+9 |
| Test Results | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-3721/8/testReport/ |
| Max. process+thread count | 4109 (vs. ulimit of 30000) |
| modules | C: hbase-server U: hbase-server |
| Console output | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-3721/8/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.
--
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.
To unsubscribe, e-mail: issues-unsubscribe@hbase.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [hbase] wchevreuil commented on a change in pull request #3721: HBASE-26326 CreateTableProcedure fails when FileBasedStoreFileTracker…
Posted by GitBox <gi...@apache.org>.
wchevreuil commented on a change in pull request #3721:
URL: https://github.com/apache/hbase/pull/3721#discussion_r726129797
##########
File path: hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/storefiletracker/StoreFileTrackerFactory.java
##########
@@ -158,12 +158,12 @@ static StoreFileTrackerBase createForMigration(Configuration conf, String config
return ReflectionUtils.newInstance(tracker, conf, isPrimaryReplica, ctx);
}
- public static void persistTrackerConfig(Configuration conf, TableDescriptorBuilder builder) {
- TableDescriptor tableDescriptor = builder.build();
- ColumnFamilyDescriptor cfDesc = tableDescriptor.getColumnFamilies()[0];
- StoreContext context = StoreContext.getBuilder().withColumnFamilyDescriptor(cfDesc).build();
- StoreFileTracker tracker = StoreFileTrackerFactory.create(conf, true, context);
- tracker.persistConfiguration(builder);
+ public static TableDescriptor updateDescriptor(Configuration conf, TableDescriptor descriptor) {
Review comment:
Ack
--
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.
To unsubscribe, e-mail: issues-unsubscribe@hbase.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [hbase] wchevreuil edited a comment on pull request #3721: HBASE-26326 CreateTableProcedure fails when FileBasedStoreFileTracker…
Posted by GitBox <gi...@apache.org>.
wchevreuil edited a comment on pull request #3721:
URL: https://github.com/apache/hbase/pull/3721#issuecomment-937818470
> The TestStoreFileTracker.testHasPersistConfiguration method is used to make sure that all the implementation classes have the correct static method.
My problem with this approach is that we then don't have an intuitive interface that clearly states what should be implemented, requiring this extra hack to enforce its rules, at test execution time only. I would rather have a more strict interface that it's easier for developer to understand what is required to be implemented (the original approach), or, since this seems to be an issue exclusively for the FILE SFT impl, let it deal with the config mode on its own (the extra check for missing info in the passed StoreContext implemented on last commit). WDYT?
> For the naming change, updateDescriptor seems too general, what is the reason for changing the method name? I do not mean we can not change it, just want to know the reason.
While going through this, I thought the `persistConfiguration` confusing. Even though I was the author of this method on few commits back, now whenever I was reading it, I was misled to think it was persisting the configuration somewhere, when in reality it was just updating the passed descriptor builder with additional configs needed by individual SFT impl. As it seems `updateDescriptor` still sounds confusing, let me try something else. How about `updateWithTrackerConfigs`?
--
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.
To unsubscribe, e-mail: issues-unsubscribe@hbase.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [hbase] wchevreuil commented on a change in pull request #3721: HBASE-26326 CreateTableProcedure fails when FileBasedStoreFileTracker…
Posted by GitBox <gi...@apache.org>.
wchevreuil commented on a change in pull request #3721:
URL: https://github.com/apache/hbase/pull/3721#discussion_r725136101
##########
File path: hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/storefiletracker/StoreFileTrackerFactory.java
##########
@@ -158,12 +166,12 @@ static StoreFileTrackerBase createForMigration(Configuration conf, String config
return ReflectionUtils.newInstance(tracker, conf, isPrimaryReplica, ctx);
}
- public static void persistTrackerConfig(Configuration conf, TableDescriptorBuilder builder) {
+ public static void updateDescriptor(Configuration conf, TableDescriptorBuilder builder) {
TableDescriptor tableDescriptor = builder.build();
ColumnFamilyDescriptor cfDesc = tableDescriptor.getColumnFamilies()[0];
StoreContext context = StoreContext.getBuilder().withColumnFamilyDescriptor(cfDesc).build();
- StoreFileTracker tracker = StoreFileTrackerFactory.create(conf, true, context);
- tracker.persistConfiguration(builder);
+ StoreFileTracker tracker = StoreFileTrackerFactory.getInstance(conf, true, context);
Review comment:
So the if check needs to be performed by the SFT impls, as those are the ones who knows what configs they should check for. Changed the method to receive and return TableDescriptor, as suggested.
--
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.
To unsubscribe, e-mail: issues-unsubscribe@hbase.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [hbase] Apache9 commented on pull request #3721: HBASE-26326 CreateTableProcedure fails when FileBasedStoreFileTracker…
Posted by GitBox <gi...@apache.org>.
Apache9 commented on pull request #3721:
URL: https://github.com/apache/hbase/pull/3721#issuecomment-937331616
--
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.
To unsubscribe, e-mail: issues-unsubscribe@hbase.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [hbase] Apache-HBase commented on pull request #3721: HBASE-26326 CreateTableProcedure fails when FileBasedStoreFileTracker…
Posted by GitBox <gi...@apache.org>.
Apache-HBase commented on pull request #3721:
URL: https://github.com/apache/hbase/pull/3721#issuecomment-937635219
--
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.
To unsubscribe, e-mail: issues-unsubscribe@hbase.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [hbase] Apache-HBase commented on pull request #3721: HBASE-26326 CreateTableProcedure fails when FileBasedStoreFileTracker…
Posted by GitBox <gi...@apache.org>.
Apache-HBase commented on pull request #3721:
URL: https://github.com/apache/hbase/pull/3721#issuecomment-937887816
:confetti_ball: **+1 overall**
| Vote | Subsystem | Runtime | Comment |
|:----:|----------:|--------:|:--------|
| +0 :ok: | reexec | 0m 26s | 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 _ |
||| _ HBASE-26067 Compile Tests _ |
| +1 :green_heart: | mvninstall | 4m 34s | HBASE-26067 passed |
| +1 :green_heart: | compile | 1m 19s | HBASE-26067 passed |
| +1 :green_heart: | shadedjars | 8m 52s | branch has no errors when building our shaded downstream artifacts. |
| +1 :green_heart: | javadoc | 0m 43s | HBASE-26067 passed |
| -0 :warning: | patch | 9m 45s | Used diff version of patch file. Binary files and potentially other changes not applied. Please rebase and squash commits if necessary. |
||| _ Patch Compile Tests _ |
| +1 :green_heart: | mvninstall | 4m 32s | the patch passed |
| +1 :green_heart: | compile | 1m 18s | the patch passed |
| +1 :green_heart: | javac | 1m 18s | the patch passed |
| +1 :green_heart: | shadedjars | 8m 35s | patch has no errors when building our shaded downstream artifacts. |
| +1 :green_heart: | javadoc | 0m 43s | the patch passed |
||| _ Other Tests _ |
| +1 :green_heart: | unit | 146m 55s | hbase-server in the patch passed. |
| | | 180m 4s | |
| 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-3721/9/artifact/yetus-jdk11-hadoop3-check/output/Dockerfile |
| GITHUB PR | https://github.com/apache/hbase/pull/3721 |
| Optional Tests | javac javadoc unit shadedjars compile |
| uname | Linux 9aed2a6c535b 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 | HBASE-26067 / c4d7d28911 |
| Default Java | AdoptOpenJDK-11.0.10+9 |
| Test Results | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-3721/9/testReport/ |
| Max. process+thread count | 4001 (vs. ulimit of 30000) |
| modules | C: hbase-server U: hbase-server |
| Console output | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-3721/9/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.
--
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.
To unsubscribe, e-mail: issues-unsubscribe@hbase.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [hbase] Apache-HBase commented on pull request #3721: HBASE-26326 CreateTableProcedure fails when FileBasedStoreFileTracker…
Posted by GitBox <gi...@apache.org>.
Apache-HBase commented on pull request #3721:
URL: https://github.com/apache/hbase/pull/3721#issuecomment-942365977
:confetti_ball: **+1 overall**
| Vote | Subsystem | Runtime | Comment |
|:----:|----------:|--------:|:--------|
| +0 :ok: | reexec | 0m 33s | Docker mode activated. |
||| _ Prechecks _ |
| +1 :green_heart: | dupname | 0m 0s | No case conflicting files found. |
| +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. |
||| _ HBASE-26067 Compile Tests _ |
| +1 :green_heart: | mvninstall | 4m 40s | HBASE-26067 passed |
| +1 :green_heart: | compile | 4m 20s | HBASE-26067 passed |
| +1 :green_heart: | checkstyle | 1m 21s | HBASE-26067 passed |
| +1 :green_heart: | spotbugs | 2m 38s | HBASE-26067 passed |
| -0 :warning: | patch | 2m 48s | Used diff version of patch file. Binary files and potentially other changes not applied. Please rebase and squash commits if necessary. |
||| _ Patch Compile Tests _ |
| +1 :green_heart: | mvninstall | 5m 1s | the patch passed |
| +1 :green_heart: | compile | 4m 7s | the patch passed |
| +1 :green_heart: | javac | 4m 7s | the patch passed |
| -0 :warning: | checkstyle | 1m 25s | hbase-server: The patch generated 5 new + 3 unchanged - 0 fixed = 8 total (was 3) |
| +1 :green_heart: | whitespace | 0m 0s | The patch has no whitespace issues. |
| +1 :green_heart: | hadoopcheck | 24m 54s | Patch does not cause any errors with Hadoop 3.1.2 3.2.1 3.3.0. |
| +1 :green_heart: | spotbugs | 3m 8s | the patch passed |
||| _ Other Tests _ |
| +1 :green_heart: | asflicense | 0m 12s | The patch does not generate ASF License warnings. |
| | | 62m 28s | |
| 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-3721/17/artifact/yetus-general-check/output/Dockerfile |
| GITHUB PR | https://github.com/apache/hbase/pull/3721 |
| Optional Tests | dupname asflicense javac spotbugs hadoopcheck hbaseanti checkstyle compile |
| uname | Linux a60898d456e1 4.15.0-156-generic #163-Ubuntu SMP Thu Aug 19 23:31:58 UTC 2021 x86_64 x86_64 x86_64 GNU/Linux |
| Build tool | maven |
| Personality | dev-support/hbase-personality.sh |
| git revision | HBASE-26067 / c4d7d28911 |
| Default Java | AdoptOpenJDK-1.8.0_282-b08 |
| checkstyle | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-3721/17/artifact/yetus-general-check/output/diff-checkstyle-hbase-server.txt |
| Max. process+thread count | 96 (vs. ulimit of 30000) |
| modules | C: hbase-server U: hbase-server |
| Console output | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-3721/17/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.
--
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.
To unsubscribe, e-mail: issues-unsubscribe@hbase.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [hbase] Apache-HBase commented on pull request #3721: HBASE-26326 CreateTableProcedure fails when FileBasedStoreFileTracker…
Posted by GitBox <gi...@apache.org>.
Apache-HBase commented on pull request #3721:
URL: https://github.com/apache/hbase/pull/3721#issuecomment-939212190
:confetti_ball: **+1 overall**
| Vote | Subsystem | Runtime | Comment |
|:----:|----------:|--------:|:--------|
| +0 :ok: | reexec | 3m 54s | 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 _ |
||| _ HBASE-26067 Compile Tests _ |
| +1 :green_heart: | mvninstall | 4m 12s | HBASE-26067 passed |
| +1 :green_heart: | compile | 1m 11s | HBASE-26067 passed |
| +1 :green_heart: | shadedjars | 8m 18s | branch has no errors when building our shaded downstream artifacts. |
| +1 :green_heart: | javadoc | 0m 40s | HBASE-26067 passed |
| -0 :warning: | patch | 9m 11s | Used diff version of patch file. Binary files and potentially other changes not applied. Please rebase and squash commits if necessary. |
||| _ Patch Compile Tests _ |
| +1 :green_heart: | mvninstall | 4m 14s | the patch passed |
| +1 :green_heart: | compile | 1m 11s | the patch passed |
| +1 :green_heart: | javac | 1m 11s | the patch passed |
| +1 :green_heart: | shadedjars | 8m 7s | patch has no errors when building our shaded downstream artifacts. |
| +1 :green_heart: | javadoc | 0m 40s | the patch passed |
||| _ Other Tests _ |
| +1 :green_heart: | unit | 142m 11s | hbase-server in the patch passed. |
| | | 177m 3s | |
| 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-3721/14/artifact/yetus-jdk11-hadoop3-check/output/Dockerfile |
| GITHUB PR | https://github.com/apache/hbase/pull/3721 |
| Optional Tests | javac javadoc unit shadedjars compile |
| uname | Linux c2250382cca7 4.15.0-112-generic #113-Ubuntu SMP Thu Jul 9 23:41:39 UTC 2020 x86_64 x86_64 x86_64 GNU/Linux |
| Build tool | maven |
| Personality | dev-support/hbase-personality.sh |
| git revision | HBASE-26067 / c4d7d28911 |
| Default Java | AdoptOpenJDK-11.0.10+9 |
| Test Results | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-3721/14/testReport/ |
| Max. process+thread count | 3919 (vs. ulimit of 30000) |
| modules | C: hbase-server U: hbase-server |
| Console output | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-3721/14/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.
--
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.
To unsubscribe, e-mail: issues-unsubscribe@hbase.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [hbase] Apache9 commented on a change in pull request #3721: HBASE-26326 CreateTableProcedure fails when FileBasedStoreFileTracker…
Posted by GitBox <gi...@apache.org>.
Apache9 commented on a change in pull request #3721:
URL: https://github.com/apache/hbase/pull/3721#discussion_r724912488
##########
File path: hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/storefiletracker/FileBasedStoreFileTracker.java
##########
@@ -50,13 +50,15 @@
@InterfaceAudience.Private
class FileBasedStoreFileTracker extends StoreFileTrackerBase {
- private final StoreFileListFile backedFile;
+ private StoreFileListFile backedFile;
Review comment:
Just add an else block to assign it with null in the constructor :)
--
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.
To unsubscribe, e-mail: issues-unsubscribe@hbase.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [hbase] wchevreuil commented on pull request #3721: HBASE-26326 CreateTableProcedure fails when FileBasedStoreFileTracker…
Posted by GitBox <gi...@apache.org>.
wchevreuil commented on pull request #3721:
URL: https://github.com/apache/hbase/pull/3721#issuecomment-937158799
--
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.
To unsubscribe, e-mail: issues-unsubscribe@hbase.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [hbase] Apache9 commented on a change in pull request #3721: HBASE-26326 CreateTableProcedure fails when FileBasedStoreFileTracker…
Posted by GitBox <gi...@apache.org>.
Apache9 commented on a change in pull request #3721:
URL: https://github.com/apache/hbase/pull/3721#discussion_r724885509
##########
File path: hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/storefiletracker/FileBasedStoreFileTracker.java
##########
@@ -50,13 +50,15 @@
@InterfaceAudience.Private
class FileBasedStoreFileTracker extends StoreFileTrackerBase {
- private final StoreFileListFile backedFile;
+ private StoreFileListFile backedFile;
private final Map<String, StoreFileInfo> storefiles = new HashMap<>();
public FileBasedStoreFileTracker(Configuration conf, boolean isPrimaryReplica, StoreContext ctx) {
super(conf, isPrimaryReplica, ctx);
- backedFile = new StoreFileListFile(ctx);
+ if (ctx != null && ctx.getFamilyStoreDirectoryPath() != null) {
Review comment:
Just test whether ctx is null?
And please add comments here to say that, when we want to add the store tracker configs to TableDescriptor, we will create store file tracker with a null StoreContext as there is no 'Store' yet, the implementation should take care of this.
##########
File path: hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/storefiletracker/StoreFileTrackerFactory.java
##########
@@ -158,12 +166,12 @@ static StoreFileTrackerBase createForMigration(Configuration conf, String config
return ReflectionUtils.newInstance(tracker, conf, isPrimaryReplica, ctx);
}
- public static void persistTrackerConfig(Configuration conf, TableDescriptorBuilder builder) {
+ public static void updateDescriptor(Configuration conf, TableDescriptorBuilder builder) {
TableDescriptor tableDescriptor = builder.build();
ColumnFamilyDescriptor cfDesc = tableDescriptor.getColumnFamilies()[0];
StoreContext context = StoreContext.getBuilder().withColumnFamilyDescriptor(cfDesc).build();
- StoreFileTracker tracker = StoreFileTrackerFactory.create(conf, true, context);
- tracker.persistConfiguration(builder);
+ StoreFileTracker tracker = StoreFileTrackerFactory.getInstance(conf, true, context);
Review comment:
Here we should create a new Composite configration to merge the global conf and TableDescriptor?
And please just pass a null StoreContext, and add comments to mention the reason here.
##########
File path: hbase-server/src/main/java/org/apache/hadoop/hbase/master/procedure/CreateTableProcedure.java
##########
@@ -291,7 +291,7 @@ private void preCreate(final MasterProcedureEnv env)
}
TableDescriptorBuilder builder = TableDescriptorBuilder.newBuilder(tableDescriptor);
- StoreFileTrackerFactory.persistTrackerConfig(env.getMasterConfiguration(), builder);
+ StoreFileTrackerFactory.updateDescriptor(env.getMasterConfiguration(), builder);
Review comment:
Let's use updateWithTrackerConfigs?
##########
File path: hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/storefiletracker/FileBasedStoreFileTracker.java
##########
@@ -50,13 +50,15 @@
@InterfaceAudience.Private
class FileBasedStoreFileTracker extends StoreFileTrackerBase {
- private final StoreFileListFile backedFile;
+ private StoreFileListFile backedFile;
Review comment:
It could still be final I think.
##########
File path: hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/storefiletracker/StoreFileTrackerFactory.java
##########
@@ -100,13 +102,19 @@ static String getStoreFileTrackerName(Class<? extends StoreFileTracker> clazz) {
}
}
- public static StoreFileTracker create(Configuration conf, boolean isPrimaryReplica,
+ private static StoreFileTracker getInstance(Configuration conf, boolean isPrimaryReplica,
StoreContext ctx) {
Class<? extends StoreFileTracker> tracker = getTrackerClass(conf);
LOG.info("instantiating StoreFileTracker impl {}", tracker.getName());
return ReflectionUtils.newInstance(tracker, conf, isPrimaryReplica, ctx);
}
+ public static StoreFileTracker create(Configuration conf, boolean isPrimaryReplica,
Review comment:
There is no difference between this method and the method 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.
To unsubscribe, e-mail: issues-unsubscribe@hbase.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [hbase] Apache-HBase commented on pull request #3721: HBASE-26326 CreateTableProcedure fails when FileBasedStoreFileTracker…
Posted by GitBox <gi...@apache.org>.
Apache-HBase commented on pull request #3721:
URL: https://github.com/apache/hbase/pull/3721#issuecomment-938093061
:confetti_ball: **+1 overall**
| Vote | Subsystem | Runtime | Comment |
|:----:|----------:|--------:|:--------|
| +0 :ok: | reexec | 1m 6s | 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 _ |
||| _ HBASE-26067 Compile Tests _ |
| +1 :green_heart: | mvninstall | 4m 16s | HBASE-26067 passed |
| +1 :green_heart: | compile | 1m 4s | HBASE-26067 passed |
| +1 :green_heart: | shadedjars | 9m 5s | branch has no errors when building our shaded downstream artifacts. |
| +1 :green_heart: | javadoc | 0m 37s | HBASE-26067 passed |
| -0 :warning: | patch | 9m 52s | Used diff version of patch file. Binary files and potentially other changes not applied. Please rebase and squash commits if necessary. |
||| _ Patch Compile Tests _ |
| +1 :green_heart: | mvninstall | 4m 11s | the patch passed |
| +1 :green_heart: | compile | 1m 4s | the patch passed |
| +1 :green_heart: | javac | 1m 4s | the patch passed |
| +1 :green_heart: | shadedjars | 9m 5s | patch has no errors when building our shaded downstream artifacts. |
| +1 :green_heart: | javadoc | 0m 35s | the patch passed |
||| _ Other Tests _ |
| +1 :green_heart: | unit | 222m 23s | hbase-server in the patch passed. |
| | | 255m 27s | |
| 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-3721/10/artifact/yetus-jdk8-hadoop3-check/output/Dockerfile |
| GITHUB PR | https://github.com/apache/hbase/pull/3721 |
| Optional Tests | javac javadoc unit shadedjars compile |
| uname | Linux b21b4a04e92b 4.15.0-142-generic #146-Ubuntu SMP Tue Apr 13 01:11:19 UTC 2021 x86_64 x86_64 x86_64 GNU/Linux |
| Build tool | maven |
| Personality | dev-support/hbase-personality.sh |
| git revision | HBASE-26067 / c4d7d28911 |
| Default Java | AdoptOpenJDK-1.8.0_282-b08 |
| Test Results | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-3721/10/testReport/ |
| Max. process+thread count | 3764 (vs. ulimit of 30000) |
| modules | C: hbase-server U: hbase-server |
| Console output | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-3721/10/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.
--
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.
To unsubscribe, e-mail: issues-unsubscribe@hbase.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [hbase] Apache-HBase commented on pull request #3721: HBASE-26326 CreateTableProcedure fails when FileBasedStoreFileTracker…
Posted by GitBox <gi...@apache.org>.
Apache-HBase commented on pull request #3721:
URL: https://github.com/apache/hbase/pull/3721#issuecomment-941736336
:confetti_ball: **+1 overall**
| Vote | Subsystem | Runtime | Comment |
|:----:|----------:|--------:|:--------|
| +0 :ok: | reexec | 0m 26s | 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 _ |
||| _ HBASE-26067 Compile Tests _ |
| +1 :green_heart: | mvninstall | 4m 1s | HBASE-26067 passed |
| +1 :green_heart: | compile | 1m 1s | HBASE-26067 passed |
| +1 :green_heart: | shadedjars | 8m 15s | branch has no errors when building our shaded downstream artifacts. |
| +1 :green_heart: | javadoc | 0m 40s | HBASE-26067 passed |
| -0 :warning: | patch | 9m 8s | Used diff version of patch file. Binary files and potentially other changes not applied. Please rebase and squash commits if necessary. |
||| _ Patch Compile Tests _ |
| +1 :green_heart: | mvninstall | 3m 41s | the patch passed |
| +1 :green_heart: | compile | 1m 2s | the patch passed |
| +1 :green_heart: | javac | 1m 2s | the patch passed |
| +1 :green_heart: | shadedjars | 8m 23s | patch has no errors when building our shaded downstream artifacts. |
| +1 :green_heart: | javadoc | 0m 38s | the patch passed |
||| _ Other Tests _ |
| +1 :green_heart: | unit | 163m 52s | hbase-server in the patch passed. |
| | | 194m 30s | |
| 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-3721/16/artifact/yetus-jdk8-hadoop3-check/output/Dockerfile |
| GITHUB PR | https://github.com/apache/hbase/pull/3721 |
| Optional Tests | javac javadoc unit shadedjars compile |
| uname | Linux 4abcc50aff85 4.15.0-112-generic #113-Ubuntu SMP Thu Jul 9 23:41:39 UTC 2020 x86_64 x86_64 x86_64 GNU/Linux |
| Build tool | maven |
| Personality | dev-support/hbase-personality.sh |
| git revision | HBASE-26067 / c4d7d28911 |
| Default Java | AdoptOpenJDK-1.8.0_282-b08 |
| Test Results | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-3721/16/testReport/ |
| Max. process+thread count | 3942 (vs. ulimit of 30000) |
| modules | C: hbase-server U: hbase-server |
| Console output | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-3721/16/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.
--
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.
To unsubscribe, e-mail: issues-unsubscribe@hbase.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [hbase] wchevreuil commented on a change in pull request #3721: HBASE-26326 CreateTableProcedure fails when FileBasedStoreFileTracker…
Posted by GitBox <gi...@apache.org>.
wchevreuil commented on a change in pull request #3721:
URL: https://github.com/apache/hbase/pull/3721#discussion_r724909151
##########
File path: hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/storefiletracker/FileBasedStoreFileTracker.java
##########
@@ -50,13 +50,15 @@
@InterfaceAudience.Private
class FileBasedStoreFileTracker extends StoreFileTrackerBase {
- private final StoreFileListFile backedFile;
+ private StoreFileListFile backedFile;
Review comment:
If we leave it file, it gives a compile error because of the if check in the constructor.
--
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.
To unsubscribe, e-mail: issues-unsubscribe@hbase.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [hbase] joshelser commented on a change in pull request #3721: HBASE-26326 CreateTableProcedure fails when FileBasedStoreFileTracker…
Posted by GitBox <gi...@apache.org>.
joshelser commented on a change in pull request #3721:
URL: https://github.com/apache/hbase/pull/3721#discussion_r728142556
##########
File path: hbase-server/src/test/java/org/apache/hadoop/hbase/master/procedure/TestCreateTableProcedure.java
##########
@@ -105,6 +106,21 @@ public void testCreateWithTrackImpl() throws Exception {
assertEquals(trackerName, htd.getValue(TRACKER_IMPL));
}
+ @Test
+ public void testCreateWithFileBasedStoreTrackerImpl() throws Exception {
+ ProcedureExecutor<MasterProcedureEnv> procExec = getMasterProcedureExecutor();
+ procExec.getEnvironment().getMasterConfiguration().set(StoreFileTrackerFactory.TRACKER_IMPL,
+ StoreFileTrackerFactory.Trackers.FILE.name());
Review comment:
ok, cool. I didn't _think_ we ran test methods concurrently, but was stymied when I tried to quickly figure that out from the pom.xml :P
--
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.
To unsubscribe, e-mail: issues-unsubscribe@hbase.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [hbase] Apache-HBase commented on pull request #3721: HBASE-26326 CreateTableProcedure fails when FileBasedStoreFileTracker…
Posted by GitBox <gi...@apache.org>.
Apache-HBase commented on pull request #3721:
URL: https://github.com/apache/hbase/pull/3721#issuecomment-933599139
:confetti_ball: **+1 overall**
| Vote | Subsystem | Runtime | Comment |
|:----:|----------:|--------:|:--------|
| +0 :ok: | reexec | 0m 33s | Docker mode activated. |
||| _ Prechecks _ |
| +1 :green_heart: | dupname | 0m 0s | No case conflicting files found. |
| +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. |
||| _ HBASE-26067 Compile Tests _ |
| +1 :green_heart: | mvninstall | 4m 36s | HBASE-26067 passed |
| +1 :green_heart: | compile | 3m 42s | HBASE-26067 passed |
| +1 :green_heart: | checkstyle | 1m 13s | HBASE-26067 passed |
| +1 :green_heart: | spotbugs | 2m 19s | HBASE-26067 passed |
||| _ Patch Compile Tests _ |
| +1 :green_heart: | mvninstall | 4m 12s | the patch passed |
| +1 :green_heart: | compile | 3m 36s | the patch passed |
| +1 :green_heart: | javac | 3m 36s | the patch passed |
| +1 :green_heart: | checkstyle | 1m 6s | the patch passed |
| +1 :green_heart: | whitespace | 0m 0s | The patch has no whitespace issues. |
| +1 :green_heart: | hadoopcheck | 20m 59s | Patch does not cause any errors with Hadoop 3.1.2 3.2.1 3.3.0. |
| +1 :green_heart: | spotbugs | 2m 30s | the patch passed |
||| _ Other Tests _ |
| +1 :green_heart: | asflicense | 0m 15s | The patch does not generate ASF License warnings. |
| | | 54m 21s | |
| 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-3721/1/artifact/yetus-general-check/output/Dockerfile |
| GITHUB PR | https://github.com/apache/hbase/pull/3721 |
| Optional Tests | dupname asflicense javac spotbugs hadoopcheck hbaseanti checkstyle compile |
| uname | Linux f5fbd9c17ac5 4.15.0-156-generic #163-Ubuntu SMP Thu Aug 19 23:31:58 UTC 2021 x86_64 x86_64 x86_64 GNU/Linux |
| Build tool | maven |
| Personality | dev-support/hbase-personality.sh |
| git revision | HBASE-26067 / ab03ee1e97 |
| Default Java | AdoptOpenJDK-1.8.0_282-b08 |
| Max. process+thread count | 96 (vs. ulimit of 30000) |
| modules | C: hbase-server U: hbase-server |
| Console output | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-3721/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.
--
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.
To unsubscribe, e-mail: issues-unsubscribe@hbase.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [hbase] wchevreuil commented on pull request #3721: HBASE-26326 CreateTableProcedure fails when FileBasedStoreFileTracker…
Posted by GitBox <gi...@apache.org>.
wchevreuil commented on pull request #3721:
URL: https://github.com/apache/hbase/pull/3721#issuecomment-934457567
> A possible way is to pass in a null StoreContext, and the implementation class should check whether the StoreContext is null, if so, then it should construct itself in the 'persist store tracker configurations' mode.
That could be an option, despite looking a bit hacky. Alternatively, we could ensure init() is called prior to start adding/loading/replacing store files. WDYT?
> I think the latter one will be cleaner but not sure if we can force the implementation class has a static method at compile time.
That was exactly my first idea, but the lack of enforcement of static methods on implementations is a problem, IMO, which led me to the alternative solution.
> But at least, we could introduce a UT for checking this.
I believe the UT I had added would catch this problem.
--
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.
To unsubscribe, e-mail: issues-unsubscribe@hbase.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [hbase] Apache-HBase commented on pull request #3721: HBASE-26326 CreateTableProcedure fails when FileBasedStoreFileTracker…
Posted by GitBox <gi...@apache.org>.
Apache-HBase commented on pull request #3721:
URL: https://github.com/apache/hbase/pull/3721#issuecomment-940125540
:confetti_ball: **+1 overall**
| Vote | Subsystem | Runtime | Comment |
|:----:|----------:|--------:|:--------|
| +0 :ok: | reexec | 3m 54s | Docker mode activated. |
||| _ Prechecks _ |
| +1 :green_heart: | dupname | 0m 0s | No case conflicting files found. |
| +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. |
||| _ HBASE-26067 Compile Tests _ |
| +1 :green_heart: | mvninstall | 3m 43s | HBASE-26067 passed |
| +1 :green_heart: | compile | 3m 11s | HBASE-26067 passed |
| +1 :green_heart: | checkstyle | 1m 2s | HBASE-26067 passed |
| +1 :green_heart: | spotbugs | 2m 3s | HBASE-26067 passed |
| -0 :warning: | patch | 2m 11s | Used diff version of patch file. Binary files and potentially other changes not applied. Please rebase and squash commits if necessary. |
||| _ Patch Compile Tests _ |
| +1 :green_heart: | mvninstall | 3m 42s | the patch passed |
| +1 :green_heart: | compile | 3m 9s | the patch passed |
| +1 :green_heart: | javac | 3m 9s | the patch passed |
| +1 :green_heart: | checkstyle | 1m 3s | the patch passed |
| +1 :green_heart: | whitespace | 0m 0s | The patch has no whitespace issues. |
| +1 :green_heart: | hadoopcheck | 18m 19s | Patch does not cause any errors with Hadoop 3.1.2 3.2.1 3.3.0. |
| +1 :green_heart: | spotbugs | 2m 17s | the patch passed |
||| _ Other Tests _ |
| +1 :green_heart: | asflicense | 0m 13s | The patch does not generate ASF License warnings. |
| | | 50m 8s | |
| 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-3721/15/artifact/yetus-general-check/output/Dockerfile |
| GITHUB PR | https://github.com/apache/hbase/pull/3721 |
| Optional Tests | dupname asflicense javac spotbugs hadoopcheck hbaseanti checkstyle compile |
| uname | Linux 72ff5314fb51 4.15.0-156-generic #163-Ubuntu SMP Thu Aug 19 23:31:58 UTC 2021 x86_64 x86_64 x86_64 GNU/Linux |
| Build tool | maven |
| Personality | dev-support/hbase-personality.sh |
| git revision | HBASE-26067 / c4d7d28911 |
| Default Java | AdoptOpenJDK-1.8.0_282-b08 |
| Max. process+thread count | 95 (vs. ulimit of 30000) |
| modules | C: hbase-server U: hbase-server |
| Console output | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-3721/15/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.
--
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.
To unsubscribe, e-mail: issues-unsubscribe@hbase.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [hbase] Apache9 commented on a change in pull request #3721: HBASE-26326 CreateTableProcedure fails when FileBasedStoreFileTracker…
Posted by GitBox <gi...@apache.org>.
Apache9 commented on a change in pull request #3721:
URL: https://github.com/apache/hbase/pull/3721#discussion_r726098183
##########
File path: hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/storefiletracker/StoreFileTrackerBase.java
##########
@@ -83,10 +84,13 @@ public final void replace(Collection<StoreFileInfo> compactedFiles,
}
@Override
- public void persistConfiguration(TableDescriptorBuilder builder) {
- if (StringUtils.isEmpty(builder.getValue(TRACKER_IMPL))) {
+ public TableDescriptor updateWithTrackerConfigs(TableDescriptor descriptor) {
Review comment:
I think here we could pass in the TableDescriptorBuilder?
##########
File path: hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/storefiletracker/StoreFileTrackerFactory.java
##########
@@ -158,12 +158,12 @@ static StoreFileTrackerBase createForMigration(Configuration conf, String config
return ReflectionUtils.newInstance(tracker, conf, isPrimaryReplica, ctx);
}
- public static void persistTrackerConfig(Configuration conf, TableDescriptorBuilder builder) {
- TableDescriptor tableDescriptor = builder.build();
- ColumnFamilyDescriptor cfDesc = tableDescriptor.getColumnFamilies()[0];
- StoreContext context = StoreContext.getBuilder().withColumnFamilyDescriptor(cfDesc).build();
- StoreFileTracker tracker = StoreFileTrackerFactory.create(conf, true, context);
- tracker.persistConfiguration(builder);
+ public static TableDescriptor updateDescriptor(Configuration conf, TableDescriptor descriptor) {
+ //CreateTableProcedure needs to instantiate the configured SFT impl, in order to update table
+ //descriptors with the SFT impl specific configs. By the time this happens, the table has no
+ //regions nor stores yet, so it can't create a proper StoreContext.
+ StoreFileTracker tracker = StoreFileTrackerFactory.create(conf, true, null);
Review comment:
I think here we could just check whether the TableDescriptor already has TRACKER_IMPL set, if so just return to original TableDescriptor, otherwise, we initialize the store file tracker, and call its updateWithTrackerConfigs method.
##########
File path: hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/storefiletracker/StoreFileTrackerFactory.java
##########
@@ -158,12 +158,12 @@ static StoreFileTrackerBase createForMigration(Configuration conf, String config
return ReflectionUtils.newInstance(tracker, conf, isPrimaryReplica, ctx);
}
- public static void persistTrackerConfig(Configuration conf, TableDescriptorBuilder builder) {
- TableDescriptor tableDescriptor = builder.build();
- ColumnFamilyDescriptor cfDesc = tableDescriptor.getColumnFamilies()[0];
- StoreContext context = StoreContext.getBuilder().withColumnFamilyDescriptor(cfDesc).build();
- StoreFileTracker tracker = StoreFileTrackerFactory.create(conf, true, context);
- tracker.persistConfiguration(builder);
+ public static TableDescriptor updateDescriptor(Configuration conf, TableDescriptor descriptor) {
Review comment:
Let's also name this method updateWIthTrackerConfigs?
##########
File path: hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/storefiletracker/MigrationStoreFileTracker.java
##########
@@ -89,14 +90,16 @@ void set(List<StoreFileInfo> files) {
}
@Override
- public void persistConfiguration(TableDescriptorBuilder builder) {
- super.persistConfiguration(builder);
- if (StringUtils.isEmpty(builder.getValue(SRC_IMPL))) {
+ public TableDescriptor updateWithTrackerConfigs(TableDescriptor descriptor) {
Review comment:
Oh, I just realized that, we do not need to implement this method for MigrationStoreFileTracker...
Please see HBASE-26264, we do not allow a new table to use MigrationStoreFileTracker, so here we could just throw a UnsupportOperationException.
##########
File path: hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/storefiletracker/StoreFileTrackerBase.java
##########
@@ -83,10 +84,13 @@ public final void replace(Collection<StoreFileInfo> compactedFiles,
}
@Override
- public void persistConfiguration(TableDescriptorBuilder builder) {
- if (StringUtils.isEmpty(builder.getValue(TRACKER_IMPL))) {
+ public TableDescriptor updateWithTrackerConfigs(TableDescriptor descriptor) {
+ if (StringUtils.isEmpty(descriptor.getValue(TRACKER_IMPL))) {
Review comment:
And we could do this check in StoreFileTrackerFactory, before caclling this method.
##########
File path: hbase-server/src/main/java/org/apache/hadoop/hbase/master/procedure/CreateTableProcedure.java
##########
@@ -291,8 +291,9 @@ private void preCreate(final MasterProcedureEnv env)
}
TableDescriptorBuilder builder = TableDescriptorBuilder.newBuilder(tableDescriptor);
- StoreFileTrackerFactory.persistTrackerConfig(env.getMasterConfiguration(), builder);
- tableDescriptor = builder.build();
+ tableDescriptor = StoreFileTrackerFactory.updateDescriptor(env.getMasterConfiguration(),
+ builder.build());
Review comment:
Why not just pass in the tableDescriptor? It is immutable...
--
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.
To unsubscribe, e-mail: issues-unsubscribe@hbase.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [hbase] Apache-HBase commented on pull request #3721: HBASE-26326 CreateTableProcedure fails when FileBasedStoreFileTracker…
Posted by GitBox <gi...@apache.org>.
Apache-HBase commented on pull request #3721:
URL: https://github.com/apache/hbase/pull/3721#issuecomment-937767347
:confetti_ball: **+1 overall**
| Vote | Subsystem | Runtime | Comment |
|:----:|----------:|--------:|:--------|
| +0 :ok: | reexec | 0m 26s | Docker mode activated. |
||| _ Prechecks _ |
| +1 :green_heart: | dupname | 0m 0s | No case conflicting files found. |
| +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. |
||| _ HBASE-26067 Compile Tests _ |
| +1 :green_heart: | mvninstall | 3m 59s | HBASE-26067 passed |
| +1 :green_heart: | compile | 3m 12s | HBASE-26067 passed |
| +1 :green_heart: | checkstyle | 1m 3s | HBASE-26067 passed |
| +1 :green_heart: | spotbugs | 2m 5s | HBASE-26067 passed |
| -0 :warning: | patch | 2m 14s | Used diff version of patch file. Binary files and potentially other changes not applied. Please rebase and squash commits if necessary. |
||| _ Patch Compile Tests _ |
| +1 :green_heart: | mvninstall | 3m 38s | the patch passed |
| +1 :green_heart: | compile | 3m 8s | the patch passed |
| +1 :green_heart: | javac | 3m 8s | the patch passed |
| -0 :warning: | checkstyle | 1m 2s | hbase-server: The patch generated 1 new + 0 unchanged - 0 fixed = 1 total (was 0) |
| +1 :green_heart: | whitespace | 0m 0s | The patch has no whitespace issues. |
| +1 :green_heart: | hadoopcheck | 18m 25s | Patch does not cause any errors with Hadoop 3.1.2 3.2.1 3.3.0. |
| +1 :green_heart: | spotbugs | 2m 15s | the patch passed |
||| _ Other Tests _ |
| +1 :green_heart: | asflicense | 0m 15s | The patch does not generate ASF License warnings. |
| | | 47m 18s | |
| 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-3721/9/artifact/yetus-general-check/output/Dockerfile |
| GITHUB PR | https://github.com/apache/hbase/pull/3721 |
| Optional Tests | dupname asflicense javac spotbugs hadoopcheck hbaseanti checkstyle compile |
| uname | Linux 3e24e2584c65 4.15.0-156-generic #163-Ubuntu SMP Thu Aug 19 23:31:58 UTC 2021 x86_64 x86_64 x86_64 GNU/Linux |
| Build tool | maven |
| Personality | dev-support/hbase-personality.sh |
| git revision | HBASE-26067 / c4d7d28911 |
| Default Java | AdoptOpenJDK-1.8.0_282-b08 |
| checkstyle | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-3721/9/artifact/yetus-general-check/output/diff-checkstyle-hbase-server.txt |
| Max. process+thread count | 95 (vs. ulimit of 30000) |
| modules | C: hbase-server U: hbase-server |
| Console output | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-3721/9/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.
--
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.
To unsubscribe, e-mail: issues-unsubscribe@hbase.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [hbase] Apache9 commented on pull request #3721: HBASE-26326 CreateTableProcedure fails when FileBasedStoreFileTracker…
Posted by GitBox <gi...@apache.org>.
Apache9 commented on pull request #3721:
URL: https://github.com/apache/hbase/pull/3721#issuecomment-933571085
In general I do not like that we introduce a new init method but do not call it when verifying...
I think we should try to provide more things when constructing the StoreFileTracker. The current implementations are already lazy enough as it does not actually touch the storage, it just creates some in memory objects...
So why not just pass in a RegionFileSystem for this case? Just like what we have done in the split/merge procedures.
```
/**
* Used at master side when splitting/merging regions, as we do not have a Store, thus no
* StoreContext at master side.
*/
public static StoreFileTracker create(Configuration conf, TableDescriptor td,
ColumnFamilyDescriptor cfd, HRegionFileSystem regionFs) {
StoreContext ctx =
StoreContext.getBuilder().withColumnFamilyDescriptor(cfd).withRegionFileSystem(regionFs)
.withFamilyStoreDirectoryPath(regionFs.getStoreDir(cfd.getNameAsString())).build();
return StoreFileTrackerFactory.create(mergeConfigurations(conf, td, cfd), true, ctx);
}
```
Thanks.
--
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.
To unsubscribe, e-mail: issues-unsubscribe@hbase.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [hbase] Apache-HBase commented on pull request #3721: HBASE-26326 CreateTableProcedure fails when FileBasedStoreFileTracker…
Posted by GitBox <gi...@apache.org>.
Apache-HBase commented on pull request #3721:
URL: https://github.com/apache/hbase/pull/3721#issuecomment-939151885
:broken_heart: **-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 _ |
||| _ HBASE-26067 Compile Tests _ |
| +1 :green_heart: | mvninstall | 4m 40s | HBASE-26067 passed |
| +1 :green_heart: | compile | 1m 13s | HBASE-26067 passed |
| +1 :green_heart: | shadedjars | 8m 15s | branch has no errors when building our shaded downstream artifacts. |
| +1 :green_heart: | javadoc | 0m 45s | HBASE-26067 passed |
| -0 :warning: | patch | 9m 12s | Used diff version of patch file. Binary files and potentially other changes not applied. Please rebase and squash commits if necessary. |
||| _ Patch Compile Tests _ |
| +1 :green_heart: | mvninstall | 4m 18s | the patch passed |
| +1 :green_heart: | compile | 1m 14s | the patch passed |
| +1 :green_heart: | javac | 1m 14s | the patch passed |
| +1 :green_heart: | shadedjars | 8m 12s | patch has no errors when building our shaded downstream artifacts. |
| +1 :green_heart: | javadoc | 0m 40s | the patch passed |
||| _ Other Tests _ |
| -1 :x: | unit | 142m 31s | hbase-server in the patch failed. |
| | | 174m 47s | |
| 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-3721/13/artifact/yetus-jdk11-hadoop3-check/output/Dockerfile |
| GITHUB PR | https://github.com/apache/hbase/pull/3721 |
| Optional Tests | javac javadoc unit shadedjars compile |
| uname | Linux 0b951fbc0415 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 | HBASE-26067 / c4d7d28911 |
| Default Java | AdoptOpenJDK-11.0.10+9 |
| unit | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-3721/13/artifact/yetus-jdk11-hadoop3-check/output/patch-unit-hbase-server.txt |
| Test Results | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-3721/13/testReport/ |
| Max. process+thread count | 4129 (vs. ulimit of 30000) |
| modules | C: hbase-server U: hbase-server |
| Console output | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-3721/13/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.
--
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.
To unsubscribe, e-mail: issues-unsubscribe@hbase.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [hbase] Apache-HBase commented on pull request #3721: HBASE-26326 CreateTableProcedure fails when FileBasedStoreFileTracker…
Posted by GitBox <gi...@apache.org>.
Apache-HBase commented on pull request #3721:
URL: https://github.com/apache/hbase/pull/3721#issuecomment-942561764
:confetti_ball: **+1 overall**
| Vote | Subsystem | Runtime | Comment |
|:----:|----------:|--------:|:--------|
| +0 :ok: | reexec | 1m 18s | 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 _ |
||| _ HBASE-26067 Compile Tests _ |
| +1 :green_heart: | mvninstall | 4m 17s | HBASE-26067 passed |
| +1 :green_heart: | compile | 1m 4s | HBASE-26067 passed |
| +1 :green_heart: | shadedjars | 9m 16s | branch has no errors when building our shaded downstream artifacts. |
| +1 :green_heart: | javadoc | 0m 36s | HBASE-26067 passed |
| -0 :warning: | patch | 10m 2s | Used diff version of patch file. Binary files and potentially other changes not applied. Please rebase and squash commits if necessary. |
||| _ Patch Compile Tests _ |
| +1 :green_heart: | mvninstall | 4m 8s | the patch passed |
| +1 :green_heart: | compile | 1m 2s | the patch passed |
| +1 :green_heart: | javac | 1m 2s | the patch passed |
| +1 :green_heart: | shadedjars | 9m 8s | patch has no errors when building our shaded downstream artifacts. |
| +1 :green_heart: | javadoc | 0m 41s | the patch passed |
||| _ Other Tests _ |
| +1 :green_heart: | unit | 220m 33s | hbase-server in the patch passed. |
| | | 253m 52s | |
| 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-3721/17/artifact/yetus-jdk8-hadoop3-check/output/Dockerfile |
| GITHUB PR | https://github.com/apache/hbase/pull/3721 |
| Optional Tests | javac javadoc unit shadedjars compile |
| uname | Linux bafde08461d1 4.15.0-143-generic #147-Ubuntu SMP Wed Apr 14 16:10:11 UTC 2021 x86_64 x86_64 x86_64 GNU/Linux |
| Build tool | maven |
| Personality | dev-support/hbase-personality.sh |
| git revision | HBASE-26067 / c4d7d28911 |
| Default Java | AdoptOpenJDK-1.8.0_282-b08 |
| Test Results | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-3721/17/testReport/ |
| Max. process+thread count | 3316 (vs. ulimit of 30000) |
| modules | C: hbase-server U: hbase-server |
| Console output | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-3721/17/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.
--
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.
To unsubscribe, e-mail: issues-unsubscribe@hbase.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [hbase] wchevreuil commented on pull request #3721: HBASE-26326 CreateTableProcedure fails when FileBasedStoreFileTracker…
Posted by GitBox <gi...@apache.org>.
wchevreuil commented on pull request #3721:
URL: https://github.com/apache/hbase/pull/3721#issuecomment-937584379
> I just rebased [HBASE-26067](https://issues.apache.org/jira/browse/HBASE-26067) against the newest master branch, and seems GitHub can not identify this type of rebase so it will show all the commits in the PR. You can do a cherry-pick to the newest [HBASE-26067](https://issues.apache.org/jira/browse/HBASE-26067) and then do a force push to your branch.
>
> Sorry for the inconvenience. In the future I will shout before doing the rebase to let others know.
No problem, just wanted to make sure on the reason. Let me do the cherry-pick.
--
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.
To unsubscribe, e-mail: issues-unsubscribe@hbase.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [hbase] Apache9 commented on a change in pull request #3721: HBASE-26326 CreateTableProcedure fails when FileBasedStoreFileTracker…
Posted by GitBox <gi...@apache.org>.
Apache9 commented on a change in pull request #3721:
URL: https://github.com/apache/hbase/pull/3721#discussion_r726131755
##########
File path: hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/storefiletracker/MigrationStoreFileTracker.java
##########
@@ -89,14 +90,16 @@ void set(List<StoreFileInfo> files) {
}
@Override
- public void persistConfiguration(TableDescriptorBuilder builder) {
- super.persistConfiguration(builder);
- if (StringUtils.isEmpty(builder.getValue(SRC_IMPL))) {
+ public TableDescriptor updateWithTrackerConfigs(TableDescriptor descriptor) {
Review comment:
If we need this there, we could change it back in the PR for HBASE-26328 I think. Maybe in the PR for HBASE-26328 we need to modify these methods again since we have new requirements, but anyway, let's keep it simple for now. WDYT?
--
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.
To unsubscribe, e-mail: issues-unsubscribe@hbase.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [hbase] wchevreuil commented on pull request #3721: HBASE-26326 CreateTableProcedure fails when FileBasedStoreFileTracker…
Posted by GitBox <gi...@apache.org>.
wchevreuil commented on pull request #3721:
URL: https://github.com/apache/hbase/pull/3721#issuecomment-937818470
> The TestStoreFileTracker.testHasPersistConfiguration method is used to make sure that all the implementation classes have the correct static method.
My problem with this approach is that we then don't have an intuitive interface that clearly states what should be implemented, requiring this extra hack to enforce its rules, at test execution time only. I would rather have a more strict interface that it's easier for developer to understand what is required to be implemented (the original approach), or, since this seems to be an issue exclusively for the FILE SFT impl, let it deal with the config mode on its own (the extra check for missing info in the passed StoreContext implemented on last commit). WDYT?
> For the naming change, updateDescriptor seems too general, what is the reason for changing the method name? I do not mean we can not change it, just want to know the reason.
While going through this, I thought the `persistConfiguration` confusing. Even though I was the author of this method on few commits back, now whenever I was reading it, I was misled to think it was persisting the configuration somewhere, when in reality it was just updating the passed descriptor builder with additional configs needed by individual SFT impl. As it seems `updateDescriptor` still sounds confusing, let me try something else. How about `updateWithTrackerConfigs`?
--
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.
To unsubscribe, e-mail: issues-unsubscribe@hbase.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [hbase] Apache-HBase commented on pull request #3721: HBASE-26326 CreateTableProcedure fails when FileBasedStoreFileTracker…
Posted by GitBox <gi...@apache.org>.
Apache-HBase commented on pull request #3721:
URL: https://github.com/apache/hbase/pull/3721#issuecomment-940360052
:confetti_ball: **+1 overall**
| Vote | Subsystem | Runtime | Comment |
|:----:|----------:|--------:|:--------|
| +0 :ok: | reexec | 1m 13s | 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 _ |
||| _ HBASE-26067 Compile Tests _ |
| +1 :green_heart: | mvninstall | 4m 27s | HBASE-26067 passed |
| +1 :green_heart: | compile | 1m 5s | HBASE-26067 passed |
| +1 :green_heart: | shadedjars | 9m 6s | branch has no errors when building our shaded downstream artifacts. |
| +1 :green_heart: | javadoc | 0m 38s | HBASE-26067 passed |
| -0 :warning: | patch | 9m 54s | Used diff version of patch file. Binary files and potentially other changes not applied. Please rebase and squash commits if necessary. |
||| _ Patch Compile Tests _ |
| +1 :green_heart: | mvninstall | 4m 8s | the patch passed |
| +1 :green_heart: | compile | 1m 4s | the patch passed |
| +1 :green_heart: | javac | 1m 4s | the patch passed |
| +1 :green_heart: | shadedjars | 9m 20s | patch has no errors when building our shaded downstream artifacts. |
| +1 :green_heart: | javadoc | 0m 37s | the patch passed |
||| _ Other Tests _ |
| +1 :green_heart: | unit | 227m 41s | hbase-server in the patch passed. |
| | | 261m 2s | |
| 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-3721/15/artifact/yetus-jdk8-hadoop3-check/output/Dockerfile |
| GITHUB PR | https://github.com/apache/hbase/pull/3721 |
| Optional Tests | javac javadoc unit shadedjars compile |
| uname | Linux d616149105d2 4.15.0-143-generic #147-Ubuntu SMP Wed Apr 14 16:10:11 UTC 2021 x86_64 x86_64 x86_64 GNU/Linux |
| Build tool | maven |
| Personality | dev-support/hbase-personality.sh |
| git revision | HBASE-26067 / c4d7d28911 |
| Default Java | AdoptOpenJDK-1.8.0_282-b08 |
| Test Results | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-3721/15/testReport/ |
| Max. process+thread count | 3356 (vs. ulimit of 30000) |
| modules | C: hbase-server U: hbase-server |
| Console output | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-3721/15/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.
--
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.
To unsubscribe, e-mail: issues-unsubscribe@hbase.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [hbase] wchevreuil merged pull request #3721: HBASE-26326 CreateTableProcedure fails when FileBasedStoreFileTracker…
Posted by GitBox <gi...@apache.org>.
wchevreuil merged pull request #3721:
URL: https://github.com/apache/hbase/pull/3721
--
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.
To unsubscribe, e-mail: issues-unsubscribe@hbase.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [hbase] Apache-HBase commented on pull request #3721: HBASE-26326 CreateTableProcedure fails when FileBasedStoreFileTracker…
Posted by GitBox <gi...@apache.org>.
Apache-HBase commented on pull request #3721:
URL: https://github.com/apache/hbase/pull/3721#issuecomment-941798031
:confetti_ball: **+1 overall**
| Vote | Subsystem | Runtime | Comment |
|:----:|----------:|--------:|:--------|
| +0 :ok: | reexec | 3m 3s | 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 _ |
||| _ HBASE-26067 Compile Tests _ |
| +1 :green_heart: | mvninstall | 5m 30s | HBASE-26067 passed |
| +1 :green_heart: | compile | 1m 29s | HBASE-26067 passed |
| +1 :green_heart: | shadedjars | 9m 39s | branch has no errors when building our shaded downstream artifacts. |
| +1 :green_heart: | javadoc | 0m 49s | HBASE-26067 passed |
| -0 :warning: | patch | 10m 38s | Used diff version of patch file. Binary files and potentially other changes not applied. Please rebase and squash commits if necessary. |
||| _ Patch Compile Tests _ |
| +1 :green_heart: | mvninstall | 5m 14s | the patch passed |
| +1 :green_heart: | compile | 1m 29s | the patch passed |
| +1 :green_heart: | javac | 1m 29s | the patch passed |
| +1 :green_heart: | shadedjars | 9m 42s | patch has no errors when building our shaded downstream artifacts. |
| +1 :green_heart: | javadoc | 0m 48s | the patch passed |
||| _ Other Tests _ |
| +1 :green_heart: | unit | 229m 6s | hbase-server in the patch passed. |
| | | 267m 58s | |
| 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-3721/16/artifact/yetus-jdk11-hadoop3-check/output/Dockerfile |
| GITHUB PR | https://github.com/apache/hbase/pull/3721 |
| Optional Tests | javac javadoc unit shadedjars compile |
| uname | Linux b09f1fcf50ef 4.15.0-142-generic #146-Ubuntu SMP Tue Apr 13 01:11:19 UTC 2021 x86_64 x86_64 x86_64 GNU/Linux |
| Build tool | maven |
| Personality | dev-support/hbase-personality.sh |
| git revision | HBASE-26067 / c4d7d28911 |
| Default Java | AdoptOpenJDK-11.0.10+9 |
| Test Results | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-3721/16/testReport/ |
| Max. process+thread count | 3220 (vs. ulimit of 30000) |
| modules | C: hbase-server U: hbase-server |
| Console output | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-3721/16/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.
--
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.
To unsubscribe, e-mail: issues-unsubscribe@hbase.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [hbase] Apache9 commented on pull request #3721: HBASE-26326 CreateTableProcedure fails when FileBasedStoreFileTracker…
Posted by GitBox <gi...@apache.org>.
Apache9 commented on pull request #3721:
URL: https://github.com/apache/hbase/pull/3721#issuecomment-934466509
The UT I mean is to check whether we have the static method for each implementation class. This is possible.
I could provide an example later.
I think this will be cleaner way. I also want to introduce a init method in the past when implementing migration store file tracker, as we will not always call load when migrating, but finally I just added a check in the StoreFileListFile, to avoid adding extra methods. For me I prefer we keep the interface simple, unless there are no other choices.
WDYT?
Thanks.
--
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.
To unsubscribe, e-mail: issues-unsubscribe@hbase.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [hbase] wchevreuil commented on a change in pull request #3721: HBASE-26326 CreateTableProcedure fails when FileBasedStoreFileTracker…
Posted by GitBox <gi...@apache.org>.
wchevreuil commented on a change in pull request #3721:
URL: https://github.com/apache/hbase/pull/3721#discussion_r727866389
##########
File path: hbase-server/src/test/java/org/apache/hadoop/hbase/master/procedure/TestCreateTableProcedure.java
##########
@@ -105,6 +106,21 @@ public void testCreateWithTrackImpl() throws Exception {
assertEquals(trackerName, htd.getValue(TRACKER_IMPL));
}
+ @Test
+ public void testCreateWithFileBasedStoreTrackerImpl() throws Exception {
+ ProcedureExecutor<MasterProcedureEnv> procExec = getMasterProcedureExecutor();
+ procExec.getEnvironment().getMasterConfiguration().set(StoreFileTrackerFactory.TRACKER_IMPL,
+ StoreFileTrackerFactory.Trackers.FILE.name());
Review comment:
Yes, just checked now and found out that this relies on a static HBaseTestingUtil variable declared in the parent TestTableDDLProcedureBase class, so any concurrent sub-classes instances of that could end up picking that config.
I don't think this would be a problem for other tests, though, since none of those are validated store file tracking work itself, and whatever is being tested elsewhere shouldn't break because of this setting (if it does break some other test, then it's probably a regression we are introducing here).
The only potential problem is if both this testCreateWithFileBasedStoreTrackerImpl and testCreateWithTrackImpl test methods run concurrently, but I don't think that happens for methods within the same test class.
--
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.
To unsubscribe, e-mail: issues-unsubscribe@hbase.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [hbase] wchevreuil commented on a change in pull request #3721: HBASE-26326 CreateTableProcedure fails when FileBasedStoreFileTracker…
Posted by GitBox <gi...@apache.org>.
wchevreuil commented on a change in pull request #3721:
URL: https://github.com/apache/hbase/pull/3721#discussion_r727458552
##########
File path: hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/storefiletracker/MigrationStoreFileTracker.java
##########
@@ -89,14 +90,16 @@ void set(List<StoreFileInfo> files) {
}
@Override
- public void persistConfiguration(TableDescriptorBuilder builder) {
- super.persistConfiguration(builder);
- if (StringUtils.isEmpty(builder.getValue(SRC_IMPL))) {
+ public TableDescriptor updateWithTrackerConfigs(TableDescriptor descriptor) {
Review comment:
Done.
##########
File path: hbase-server/src/test/java/org/apache/hadoop/hbase/master/procedure/TestCreateTableProcedure.java
##########
@@ -105,6 +106,21 @@ public void testCreateWithTrackImpl() throws Exception {
assertEquals(trackerName, htd.getValue(TRACKER_IMPL));
}
+ @Test
+ public void testCreateWithFileBasedStoreTrackerImpl() throws Exception {
+ ProcedureExecutor<MasterProcedureEnv> procExec = getMasterProcedureExecutor();
+ procExec.getEnvironment().getMasterConfiguration().set(StoreFileTrackerFactory.TRACKER_IMPL,
+ StoreFileTrackerFactory.Trackers.FILE.name());
Review comment:
Yes, just checked now and found out that this relies on a static HBaseTestingUtil variable declared in the parent TestTableDDLProcedureBase class, so any concurrent sub-classes instances of that could end up picking that config.
I don't think this would be a problem for other tests, though, since none of those are validated store file tracking work itself, and whatever is being tested elsewhere shouldn't break because of this setting (if it does break some other test, then it's probably a regression we are introducing here).
The only potential problem is if both this testCreateWithFileBasedStoreTrackerImpl and testCreateWithTrackImpl test methods run concurrently, but I don't think that happens for methods within the same test class.
##########
File path: hbase-server/src/test/java/org/apache/hadoop/hbase/master/procedure/TestCreateTableProcedure.java
##########
@@ -105,6 +106,21 @@ public void testCreateWithTrackImpl() throws Exception {
assertEquals(trackerName, htd.getValue(TRACKER_IMPL));
}
+ @Test
+ public void testCreateWithFileBasedStoreTrackerImpl() throws Exception {
+ ProcedureExecutor<MasterProcedureEnv> procExec = getMasterProcedureExecutor();
+ procExec.getEnvironment().getMasterConfiguration().set(StoreFileTrackerFactory.TRACKER_IMPL,
+ StoreFileTrackerFactory.Trackers.FILE.name());
Review comment:
Yes, just checked now and found out that this relies on a static HBaseTestingUtil variable declared in the parent TestTableDDLProcedureBase class, so any concurrent sub-classes instances of that could end up picking that config.
I don't think this would be a problem for other tests, though, since none of those are validating store file tracking work itself, and whatever is being tested elsewhere shouldn't break because of this setting (if it does break some other test, then it's probably a regression we are introducing here).
The only potential problem is if both this testCreateWithFileBasedStoreTrackerImpl and testCreateWithTrackImpl test methods run concurrently, but I don't think that happens for methods within the same test class.
##########
File path: hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/storefiletracker/StoreFileTracker.java
##########
@@ -75,13 +76,13 @@ void replace(Collection<StoreFileInfo> compactedFiles, Collection<StoreFileInfo>
StoreFileWriter createWriter(CreateStoreFileWriterParams params) throws IOException;
/**
- * Saves StoreFileTracker implementations specific configurations into the table descriptors.
+ * Adds StoreFileTracker implementations specific configurations into the table descriptor.
* <p/>
* This is used to avoid accidentally data loss when changing the cluster level store file tracker
* implementation, and also possible misconfiguration between master and region servers.
* <p/>
* See HBASE-26246 for more details.
* @param builder The table descriptor builder for the given table.
*/
- void persistConfiguration(TableDescriptorBuilder builder);
+ TableDescriptor updateWithTrackerConfigs(TableDescriptorBuilder builder);
Review comment:
> IMO, i'd just return the Builder back and let the caller build() when they're ready.
Yeah, I guess that would be better. WDYT, @Apache9 ?
--
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.
To unsubscribe, e-mail: issues-unsubscribe@hbase.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [hbase] Apache-HBase commented on pull request #3721: HBASE-26326 CreateTableProcedure fails when FileBasedStoreFileTracker…
Posted by GitBox <gi...@apache.org>.
Apache-HBase commented on pull request #3721:
URL: https://github.com/apache/hbase/pull/3721#issuecomment-941532157
:confetti_ball: **+1 overall**
| Vote | Subsystem | Runtime | Comment |
|:----:|----------:|--------:|:--------|
| +0 :ok: | reexec | 0m 27s | Docker mode activated. |
||| _ Prechecks _ |
| +1 :green_heart: | dupname | 0m 0s | No case conflicting files found. |
| +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. |
||| _ HBASE-26067 Compile Tests _ |
| +1 :green_heart: | mvninstall | 3m 42s | HBASE-26067 passed |
| +1 :green_heart: | compile | 3m 7s | HBASE-26067 passed |
| +1 :green_heart: | checkstyle | 1m 1s | HBASE-26067 passed |
| +1 :green_heart: | spotbugs | 2m 0s | HBASE-26067 passed |
| -0 :warning: | patch | 2m 9s | Used diff version of patch file. Binary files and potentially other changes not applied. Please rebase and squash commits if necessary. |
||| _ Patch Compile Tests _ |
| +1 :green_heart: | mvninstall | 3m 37s | the patch passed |
| +1 :green_heart: | compile | 3m 13s | the patch passed |
| +1 :green_heart: | javac | 3m 13s | the patch passed |
| -0 :warning: | checkstyle | 1m 1s | hbase-server: The patch generated 3 new + 3 unchanged - 0 fixed = 6 total (was 3) |
| +1 :green_heart: | whitespace | 0m 0s | The patch has no whitespace issues. |
| +1 :green_heart: | hadoopcheck | 18m 16s | Patch does not cause any errors with Hadoop 3.1.2 3.2.1 3.3.0. |
| +1 :green_heart: | spotbugs | 2m 15s | the patch passed |
||| _ Other Tests _ |
| +1 :green_heart: | asflicense | 0m 13s | The patch does not generate ASF License warnings. |
| | | 46m 30s | |
| 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-3721/16/artifact/yetus-general-check/output/Dockerfile |
| GITHUB PR | https://github.com/apache/hbase/pull/3721 |
| Optional Tests | dupname asflicense javac spotbugs hadoopcheck hbaseanti checkstyle compile |
| uname | Linux 4d88bbff0eed 4.15.0-156-generic #163-Ubuntu SMP Thu Aug 19 23:31:58 UTC 2021 x86_64 x86_64 x86_64 GNU/Linux |
| Build tool | maven |
| Personality | dev-support/hbase-personality.sh |
| git revision | HBASE-26067 / c4d7d28911 |
| Default Java | AdoptOpenJDK-1.8.0_282-b08 |
| checkstyle | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-3721/16/artifact/yetus-general-check/output/diff-checkstyle-hbase-server.txt |
| Max. process+thread count | 96 (vs. ulimit of 30000) |
| modules | C: hbase-server U: hbase-server |
| Console output | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-3721/16/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.
--
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.
To unsubscribe, e-mail: issues-unsubscribe@hbase.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [hbase] Apache9 commented on pull request #3721: HBASE-26326 CreateTableProcedure fails when FileBasedStoreFileTracker…
Posted by GitBox <gi...@apache.org>.
Apache9 commented on pull request #3721:
URL: https://github.com/apache/hbase/pull/3721#issuecomment-937825943
Yes, static method inheriting is always a pain in the java world.
You find the problem and provide the PR so I think your opinion is most important. Just want to show the static method approach, do not mean we must go that way, never mind. Let me give another round of review for this PR.
> How about updateWithTrackerConfigs?
I think updateWithTrackerConfigs is fine, better than the previous two candidates.
Thanks.
--
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.
To unsubscribe, e-mail: issues-unsubscribe@hbase.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [hbase] Apache9 commented on a change in pull request #3721: HBASE-26326 CreateTableProcedure fails when FileBasedStoreFileTracker…
Posted by GitBox <gi...@apache.org>.
Apache9 commented on a change in pull request #3721:
URL: https://github.com/apache/hbase/pull/3721#discussion_r725002420
##########
File path: hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/storefiletracker/StoreFileTrackerFactory.java
##########
@@ -158,12 +166,12 @@ static StoreFileTrackerBase createForMigration(Configuration conf, String config
return ReflectionUtils.newInstance(tracker, conf, isPrimaryReplica, ctx);
}
- public static void persistTrackerConfig(Configuration conf, TableDescriptorBuilder builder) {
+ public static void updateDescriptor(Configuration conf, TableDescriptorBuilder builder) {
TableDescriptor tableDescriptor = builder.build();
ColumnFamilyDescriptor cfDesc = tableDescriptor.getColumnFamilies()[0];
StoreContext context = StoreContext.getBuilder().withColumnFamilyDescriptor(cfDesc).build();
- StoreFileTracker tracker = StoreFileTrackerFactory.create(conf, true, context);
- tracker.persistConfiguration(builder);
+ StoreFileTracker tracker = StoreFileTrackerFactory.getInstance(conf, true, context);
Review comment:
Then I think we could just add the if already set check here? And we can make this method accept a TableDescriptor instead of a TableDescriptor builder, and also return a TableDescriptor. If we found out that the TableDescriptor already has the tracker config, then we just return the TableDescriptor itself. If not, we initialize the StoreFileTracker and add its configs to the TableDescriptor, by creating a TableDescriptorBuilder and return the newly built TableDescriptor.
--
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.
To unsubscribe, e-mail: issues-unsubscribe@hbase.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [hbase] Apache9 commented on a change in pull request #3721: HBASE-26326 CreateTableProcedure fails when FileBasedStoreFileTracker…
Posted by GitBox <gi...@apache.org>.
Apache9 commented on a change in pull request #3721:
URL: https://github.com/apache/hbase/pull/3721#discussion_r725002420
##########
File path: hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/storefiletracker/StoreFileTrackerFactory.java
##########
@@ -158,12 +166,12 @@ static StoreFileTrackerBase createForMigration(Configuration conf, String config
return ReflectionUtils.newInstance(tracker, conf, isPrimaryReplica, ctx);
}
- public static void persistTrackerConfig(Configuration conf, TableDescriptorBuilder builder) {
+ public static void updateDescriptor(Configuration conf, TableDescriptorBuilder builder) {
TableDescriptor tableDescriptor = builder.build();
ColumnFamilyDescriptor cfDesc = tableDescriptor.getColumnFamilies()[0];
StoreContext context = StoreContext.getBuilder().withColumnFamilyDescriptor(cfDesc).build();
- StoreFileTracker tracker = StoreFileTrackerFactory.create(conf, true, context);
- tracker.persistConfiguration(builder);
+ StoreFileTracker tracker = StoreFileTrackerFactory.getInstance(conf, true, context);
Review comment:
Then I think we could just add the if already set check here? And we can make this method accept a TableDescriptor instead of a TableDescriptor builder, and also return a TableDescriptor. If we find out that the TableDescriptor already has the tracker config, then we just return the TableDescriptor itself. If not, we initialize the StoreFileTracker and add its configs to the TableDescriptor, by creating a TableDescriptorBuilder and return the newly built TableDescriptor.
--
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.
To unsubscribe, e-mail: issues-unsubscribe@hbase.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [hbase] wchevreuil commented on a change in pull request #3721: HBASE-26326 CreateTableProcedure fails when FileBasedStoreFileTracker…
Posted by GitBox <gi...@apache.org>.
wchevreuil commented on a change in pull request #3721:
URL: https://github.com/apache/hbase/pull/3721#discussion_r724952316
##########
File path: hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/storefiletracker/StoreFileTrackerFactory.java
##########
@@ -158,12 +166,12 @@ static StoreFileTrackerBase createForMigration(Configuration conf, String config
return ReflectionUtils.newInstance(tracker, conf, isPrimaryReplica, ctx);
}
- public static void persistTrackerConfig(Configuration conf, TableDescriptorBuilder builder) {
+ public static void updateDescriptor(Configuration conf, TableDescriptorBuilder builder) {
TableDescriptor tableDescriptor = builder.build();
ColumnFamilyDescriptor cfDesc = tableDescriptor.getColumnFamilies()[0];
StoreContext context = StoreContext.getBuilder().withColumnFamilyDescriptor(cfDesc).build();
- StoreFileTracker tracker = StoreFileTrackerFactory.create(conf, true, context);
- tracker.persistConfiguration(builder);
+ StoreFileTracker tracker = StoreFileTrackerFactory.getInstance(conf, true, context);
Review comment:
> Here we should create a new Composite configration to merge the global conf and TableDescriptor?
I think we don't really need this extra merge here, as once we have it into table descriptors, it's what will be used when opening the store. Besides, it would add extra complexity to set the newly created compound configuration into the MasterProcedureEnvironmet instance we have at CreateTableProcedure.
--
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.
To unsubscribe, e-mail: issues-unsubscribe@hbase.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [hbase] wchevreuil commented on a change in pull request #3721: HBASE-26326 CreateTableProcedure fails when FileBasedStoreFileTracker…
Posted by GitBox <gi...@apache.org>.
wchevreuil commented on a change in pull request #3721:
URL: https://github.com/apache/hbase/pull/3721#discussion_r727882607
##########
File path: hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/storefiletracker/StoreFileTracker.java
##########
@@ -75,13 +76,13 @@ void replace(Collection<StoreFileInfo> compactedFiles, Collection<StoreFileInfo>
StoreFileWriter createWriter(CreateStoreFileWriterParams params) throws IOException;
/**
- * Saves StoreFileTracker implementations specific configurations into the table descriptors.
+ * Adds StoreFileTracker implementations specific configurations into the table descriptor.
* <p/>
* This is used to avoid accidentally data loss when changing the cluster level store file tracker
* implementation, and also possible misconfiguration between master and region servers.
* <p/>
* See HBASE-26246 for more details.
* @param builder The table descriptor builder for the given table.
*/
- void persistConfiguration(TableDescriptorBuilder builder);
+ TableDescriptor updateWithTrackerConfigs(TableDescriptorBuilder builder);
Review comment:
> IMO, i'd just return the Builder back and let the caller build() when they're ready.
Yeah, I guess that would be better. WDYT, @Apache9 ?
--
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.
To unsubscribe, e-mail: issues-unsubscribe@hbase.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [hbase] Apache-HBase commented on pull request #3721: HBASE-26326 CreateTableProcedure fails when FileBasedStoreFileTracker…
Posted by GitBox <gi...@apache.org>.
Apache-HBase commented on pull request #3721:
URL: https://github.com/apache/hbase/pull/3721#issuecomment-942489990
:confetti_ball: **+1 overall**
| Vote | Subsystem | Runtime | Comment |
|:----:|----------:|--------:|:--------|
| +0 :ok: | reexec | 0m 39s | 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 _ |
||| _ HBASE-26067 Compile Tests _ |
| +1 :green_heart: | mvninstall | 5m 41s | HBASE-26067 passed |
| +1 :green_heart: | compile | 1m 34s | HBASE-26067 passed |
| +1 :green_heart: | shadedjars | 10m 20s | branch has no errors when building our shaded downstream artifacts. |
| +1 :green_heart: | javadoc | 0m 48s | HBASE-26067 passed |
| -0 :warning: | patch | 11m 19s | Used diff version of patch file. Binary files and potentially other changes not applied. Please rebase and squash commits if necessary. |
||| _ Patch Compile Tests _ |
| +1 :green_heart: | mvninstall | 5m 43s | the patch passed |
| +1 :green_heart: | compile | 1m 37s | the patch passed |
| +1 :green_heart: | javac | 1m 37s | the patch passed |
| +1 :green_heart: | shadedjars | 11m 0s | patch has no errors when building our shaded downstream artifacts. |
| +1 :green_heart: | javadoc | 0m 51s | the patch passed |
||| _ Other Tests _ |
| +1 :green_heart: | unit | 153m 22s | hbase-server in the patch passed. |
| | | 193m 38s | |
| 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-3721/17/artifact/yetus-jdk11-hadoop3-check/output/Dockerfile |
| GITHUB PR | https://github.com/apache/hbase/pull/3721 |
| Optional Tests | javac javadoc unit shadedjars compile |
| uname | Linux 9dc663f47ebb 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 | HBASE-26067 / c4d7d28911 |
| Default Java | AdoptOpenJDK-11.0.10+9 |
| Test Results | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-3721/17/testReport/ |
| Max. process+thread count | 4119 (vs. ulimit of 30000) |
| modules | C: hbase-server U: hbase-server |
| Console output | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-3721/17/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.
--
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.
To unsubscribe, e-mail: issues-unsubscribe@hbase.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [hbase] wchevreuil commented on pull request #3721: HBASE-26326 CreateTableProcedure fails when FileBasedStoreFileTracker…
Posted by GitBox <gi...@apache.org>.
wchevreuil commented on pull request #3721:
URL: https://github.com/apache/hbase/pull/3721#issuecomment-933592090
> In general I do not like that we introduce a new init method but do not call it when verifying...
It's a mean to separate instantiation of SFT impls, from actual init logic. In this particular case, we need SFT impl specific logic about setting configs before we can effectively start a SFT tracker properly.
>
> I think we should try to provide more things when constructing the StoreFileTracker. The current implementations are already lazy enough as we do not actually touch the storage, we just create some in memory objects...
>
> So why not just pass in a RegionFileSystem for this case? Just like what we have done in the split/merge procedures.
>
> ```
> /**
> * Used at master side when splitting/merging regions, as we do not have a Store, thus no
> * StoreContext at master side.
> */
> public static StoreFileTracker create(Configuration conf, TableDescriptor td,
> ColumnFamilyDescriptor cfd, HRegionFileSystem regionFs) {
> StoreContext ctx =
> StoreContext.getBuilder().withColumnFamilyDescriptor(cfd).withRegionFileSystem(regionFs)
> .withFamilyStoreDirectoryPath(regionFs.getStoreDir(cfd.getNameAsString())).build();
> return StoreFileTrackerFactory.create(mergeConfigurations(conf, td, cfd), true, ctx);
> }
> ```
>
> Thanks.
We don't have store info at CreateTable procedure time, we don't even have the regions yet by then, so how can we create a RegionFileSystem?
--
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.
To unsubscribe, e-mail: issues-unsubscribe@hbase.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [hbase] Apache-HBase commented on pull request #3721: HBASE-26326 CreateTableProcedure fails when FileBasedStoreFileTracker…
Posted by GitBox <gi...@apache.org>.
Apache-HBase commented on pull request #3721:
URL: https://github.com/apache/hbase/pull/3721#issuecomment-933759086
:confetti_ball: **+1 overall**
| Vote | Subsystem | Runtime | Comment |
|:----:|----------:|--------:|:--------|
| +0 :ok: | reexec | 1m 32s | 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 _ |
||| _ HBASE-26067 Compile Tests _ |
| +1 :green_heart: | mvninstall | 4m 24s | HBASE-26067 passed |
| +1 :green_heart: | compile | 1m 5s | HBASE-26067 passed |
| +1 :green_heart: | shadedjars | 9m 2s | branch has no errors when building our shaded downstream artifacts. |
| +1 :green_heart: | javadoc | 0m 39s | HBASE-26067 passed |
||| _ Patch Compile Tests _ |
| +1 :green_heart: | mvninstall | 4m 10s | the patch passed |
| +1 :green_heart: | compile | 1m 4s | the patch passed |
| +1 :green_heart: | javac | 1m 4s | the patch passed |
| +1 :green_heart: | shadedjars | 9m 11s | patch has no errors when building our shaded downstream artifacts. |
| +1 :green_heart: | javadoc | 0m 37s | the patch passed |
||| _ Other Tests _ |
| +1 :green_heart: | unit | 222m 10s | hbase-server in the patch passed. |
| | | 256m 0s | |
| 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-3721/1/artifact/yetus-jdk8-hadoop3-check/output/Dockerfile |
| GITHUB PR | https://github.com/apache/hbase/pull/3721 |
| Optional Tests | javac javadoc unit shadedjars compile |
| uname | Linux 818e07fa20ea 4.15.0-143-generic #147-Ubuntu SMP Wed Apr 14 16:10:11 UTC 2021 x86_64 x86_64 x86_64 GNU/Linux |
| Build tool | maven |
| Personality | dev-support/hbase-personality.sh |
| git revision | HBASE-26067 / ab03ee1e97 |
| Default Java | AdoptOpenJDK-1.8.0_282-b08 |
| Test Results | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-3721/1/testReport/ |
| Max. process+thread count | 3260 (vs. ulimit of 30000) |
| modules | C: hbase-server U: hbase-server |
| Console output | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-3721/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.
--
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.
To unsubscribe, e-mail: issues-unsubscribe@hbase.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [hbase] wchevreuil commented on pull request #3721: HBASE-26326 CreateTableProcedure fails when FileBasedStoreFileTracker…
Posted by GitBox <gi...@apache.org>.
wchevreuil commented on pull request #3721:
URL: https://github.com/apache/hbase/pull/3721#issuecomment-937591510
I'm not sure why github is showing all these additional commits, it should have the option to rebase.
--
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.
To unsubscribe, e-mail: issues-unsubscribe@hbase.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [hbase] Apache-HBase commented on pull request #3721: HBASE-26326 CreateTableProcedure fails when FileBasedStoreFileTracker…
Posted by GitBox <gi...@apache.org>.
Apache-HBase commented on pull request #3721:
URL: https://github.com/apache/hbase/pull/3721#issuecomment-938654033
:confetti_ball: **+1 overall**
| Vote | Subsystem | Runtime | Comment |
|:----:|----------:|--------:|:--------|
| +0 :ok: | reexec | 0m 28s | 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 _ |
||| _ HBASE-26067 Compile Tests _ |
| +1 :green_heart: | mvninstall | 4m 18s | HBASE-26067 passed |
| +1 :green_heart: | compile | 1m 11s | HBASE-26067 passed |
| +1 :green_heart: | shadedjars | 8m 13s | branch has no errors when building our shaded downstream artifacts. |
| +1 :green_heart: | javadoc | 0m 40s | HBASE-26067 passed |
| -0 :warning: | patch | 9m 5s | Used diff version of patch file. Binary files and potentially other changes not applied. Please rebase and squash commits if necessary. |
||| _ Patch Compile Tests _ |
| +1 :green_heart: | mvninstall | 4m 17s | the patch passed |
| +1 :green_heart: | compile | 1m 11s | the patch passed |
| +1 :green_heart: | javac | 1m 11s | the patch passed |
| +1 :green_heart: | shadedjars | 8m 12s | patch has no errors when building our shaded downstream artifacts. |
| +1 :green_heart: | javadoc | 0m 41s | the patch passed |
||| _ Other Tests _ |
| +1 :green_heart: | unit | 142m 18s | hbase-server in the patch passed. |
| | | 173m 41s | |
| 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-3721/11/artifact/yetus-jdk11-hadoop3-check/output/Dockerfile |
| GITHUB PR | https://github.com/apache/hbase/pull/3721 |
| Optional Tests | javac javadoc unit shadedjars compile |
| uname | Linux 08a59aae624d 4.15.0-112-generic #113-Ubuntu SMP Thu Jul 9 23:41:39 UTC 2020 x86_64 x86_64 x86_64 GNU/Linux |
| Build tool | maven |
| Personality | dev-support/hbase-personality.sh |
| git revision | HBASE-26067 / c4d7d28911 |
| Default Java | AdoptOpenJDK-11.0.10+9 |
| Test Results | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-3721/11/testReport/ |
| Max. process+thread count | 4200 (vs. ulimit of 30000) |
| modules | C: hbase-server U: hbase-server |
| Console output | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-3721/11/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.
--
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.
To unsubscribe, e-mail: issues-unsubscribe@hbase.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [hbase] Apache-HBase commented on pull request #3721: HBASE-26326 CreateTableProcedure fails when FileBasedStoreFileTracker…
Posted by GitBox <gi...@apache.org>.
Apache-HBase commented on pull request #3721:
URL: https://github.com/apache/hbase/pull/3721#issuecomment-937889024
:broken_heart: **-1 overall**
| Vote | Subsystem | Runtime | Comment |
|:----:|----------:|--------:|:--------|
| +0 :ok: | reexec | 0m 25s | 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 _ |
||| _ HBASE-26067 Compile Tests _ |
| +1 :green_heart: | mvninstall | 3m 42s | HBASE-26067 passed |
| +1 :green_heart: | compile | 1m 2s | HBASE-26067 passed |
| +1 :green_heart: | shadedjars | 8m 16s | branch has no errors when building our shaded downstream artifacts. |
| +1 :green_heart: | javadoc | 0m 36s | HBASE-26067 passed |
| -0 :warning: | patch | 9m 5s | Used diff version of patch file. Binary files and potentially other changes not applied. Please rebase and squash commits if necessary. |
||| _ Patch Compile Tests _ |
| +1 :green_heart: | mvninstall | 3m 48s | the patch passed |
| +1 :green_heart: | compile | 1m 1s | the patch passed |
| +1 :green_heart: | javac | 1m 1s | the patch passed |
| +1 :green_heart: | shadedjars | 8m 16s | patch has no errors when building our shaded downstream artifacts. |
| +1 :green_heart: | javadoc | 0m 37s | the patch passed |
||| _ Other Tests _ |
| -1 :x: | unit | 151m 31s | hbase-server in the patch failed. |
| | | 181m 25s | |
| 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-3721/9/artifact/yetus-jdk8-hadoop3-check/output/Dockerfile |
| GITHUB PR | https://github.com/apache/hbase/pull/3721 |
| Optional Tests | javac javadoc unit shadedjars compile |
| uname | Linux 7bbf0b6d0a82 4.15.0-112-generic #113-Ubuntu SMP Thu Jul 9 23:41:39 UTC 2020 x86_64 x86_64 x86_64 GNU/Linux |
| Build tool | maven |
| Personality | dev-support/hbase-personality.sh |
| git revision | HBASE-26067 / c4d7d28911 |
| Default Java | AdoptOpenJDK-1.8.0_282-b08 |
| unit | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-3721/9/artifact/yetus-jdk8-hadoop3-check/output/patch-unit-hbase-server.txt |
| Test Results | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-3721/9/testReport/ |
| Max. process+thread count | 4765 (vs. ulimit of 30000) |
| modules | C: hbase-server U: hbase-server |
| Console output | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-3721/9/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.
--
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.
To unsubscribe, e-mail: issues-unsubscribe@hbase.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [hbase] Apache-HBase commented on pull request #3721: HBASE-26326 CreateTableProcedure fails when FileBasedStoreFileTracker…
Posted by GitBox <gi...@apache.org>.
Apache-HBase commented on pull request #3721:
URL: https://github.com/apache/hbase/pull/3721#issuecomment-938043443
:broken_heart: **-1 overall**
| Vote | Subsystem | Runtime | Comment |
|:----:|----------:|--------:|:--------|
| +0 :ok: | reexec | 0m 28s | 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 _ |
||| _ HBASE-26067 Compile Tests _ |
| +1 :green_heart: | mvninstall | 4m 47s | HBASE-26067 passed |
| +1 :green_heart: | compile | 1m 19s | HBASE-26067 passed |
| +1 :green_heart: | shadedjars | 9m 21s | branch has no errors when building our shaded downstream artifacts. |
| +1 :green_heart: | javadoc | 0m 44s | HBASE-26067 passed |
| -0 :warning: | patch | 10m 18s | Used diff version of patch file. Binary files and potentially other changes not applied. Please rebase and squash commits if necessary. |
||| _ Patch Compile Tests _ |
| +1 :green_heart: | mvninstall | 4m 47s | the patch passed |
| +1 :green_heart: | compile | 1m 25s | the patch passed |
| +1 :green_heart: | javac | 1m 25s | the patch passed |
| +1 :green_heart: | shadedjars | 9m 12s | patch has no errors when building our shaded downstream artifacts. |
| +1 :green_heart: | javadoc | 0m 47s | the patch passed |
||| _ Other Tests _ |
| -1 :x: | unit | 150m 16s | hbase-server in the patch failed. |
| | | 185m 24s | |
| 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-3721/10/artifact/yetus-jdk11-hadoop3-check/output/Dockerfile |
| GITHUB PR | https://github.com/apache/hbase/pull/3721 |
| Optional Tests | javac javadoc unit shadedjars compile |
| uname | Linux 058111ef3872 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 | HBASE-26067 / c4d7d28911 |
| Default Java | AdoptOpenJDK-11.0.10+9 |
| unit | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-3721/10/artifact/yetus-jdk11-hadoop3-check/output/patch-unit-hbase-server.txt |
| Test Results | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-3721/10/testReport/ |
| Max. process+thread count | 4277 (vs. ulimit of 30000) |
| modules | C: hbase-server U: hbase-server |
| Console output | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-3721/10/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.
--
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.
To unsubscribe, e-mail: issues-unsubscribe@hbase.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [hbase] Apache-HBase commented on pull request #3721: HBASE-26326 CreateTableProcedure fails when FileBasedStoreFileTracker…
Posted by GitBox <gi...@apache.org>.
Apache-HBase commented on pull request #3721:
URL: https://github.com/apache/hbase/pull/3721#issuecomment-938571432
:confetti_ball: **+1 overall**
| Vote | Subsystem | Runtime | Comment |
|:----:|----------:|--------:|:--------|
| +0 :ok: | reexec | 0m 28s | Docker mode activated. |
||| _ Prechecks _ |
| +1 :green_heart: | dupname | 0m 0s | No case conflicting files found. |
| +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. |
||| _ HBASE-26067 Compile Tests _ |
| +1 :green_heart: | mvninstall | 3m 50s | HBASE-26067 passed |
| +1 :green_heart: | compile | 3m 17s | HBASE-26067 passed |
| +1 :green_heart: | checkstyle | 1m 2s | HBASE-26067 passed |
| +1 :green_heart: | spotbugs | 2m 2s | HBASE-26067 passed |
| -0 :warning: | patch | 2m 11s | Used diff version of patch file. Binary files and potentially other changes not applied. Please rebase and squash commits if necessary. |
||| _ Patch Compile Tests _ |
| +1 :green_heart: | mvninstall | 3m 39s | the patch passed |
| +1 :green_heart: | compile | 3m 14s | the patch passed |
| +1 :green_heart: | javac | 3m 14s | the patch passed |
| -0 :warning: | checkstyle | 1m 3s | hbase-server: The patch generated 1 new + 0 unchanged - 0 fixed = 1 total (was 0) |
| +1 :green_heart: | whitespace | 0m 0s | The patch has no whitespace issues. |
| +1 :green_heart: | hadoopcheck | 18m 17s | Patch does not cause any errors with Hadoop 3.1.2 3.2.1 3.3.0. |
| +1 :green_heart: | spotbugs | 2m 12s | the patch passed |
||| _ Other Tests _ |
| +1 :green_heart: | asflicense | 0m 13s | The patch does not generate ASF License warnings. |
| | | 47m 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-3721/11/artifact/yetus-general-check/output/Dockerfile |
| GITHUB PR | https://github.com/apache/hbase/pull/3721 |
| Optional Tests | dupname asflicense javac spotbugs hadoopcheck hbaseanti checkstyle compile |
| uname | Linux 62cdca96f0c3 4.15.0-156-generic #163-Ubuntu SMP Thu Aug 19 23:31:58 UTC 2021 x86_64 x86_64 x86_64 GNU/Linux |
| Build tool | maven |
| Personality | dev-support/hbase-personality.sh |
| git revision | HBASE-26067 / c4d7d28911 |
| Default Java | AdoptOpenJDK-1.8.0_282-b08 |
| checkstyle | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-3721/11/artifact/yetus-general-check/output/diff-checkstyle-hbase-server.txt |
| Max. process+thread count | 96 (vs. ulimit of 30000) |
| modules | C: hbase-server U: hbase-server |
| Console output | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-3721/11/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.
--
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.
To unsubscribe, e-mail: issues-unsubscribe@hbase.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [hbase] Apache-HBase commented on pull request #3721: HBASE-26326 CreateTableProcedure fails when FileBasedStoreFileTracker…
Posted by GitBox <gi...@apache.org>.
Apache-HBase commented on pull request #3721:
URL: https://github.com/apache/hbase/pull/3721#issuecomment-939074397
:broken_heart: **-1 overall**
| Vote | Subsystem | Runtime | Comment |
|:----:|----------:|--------:|:--------|
| +0 :ok: | reexec | 1m 6s | 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 _ |
||| _ HBASE-26067 Compile Tests _ |
| +1 :green_heart: | mvninstall | 4m 45s | HBASE-26067 passed |
| +1 :green_heart: | compile | 1m 21s | HBASE-26067 passed |
| +1 :green_heart: | shadedjars | 9m 7s | branch has no errors when building our shaded downstream artifacts. |
| +1 :green_heart: | javadoc | 0m 42s | HBASE-26067 passed |
| -0 :warning: | patch | 9m 59s | Used diff version of patch file. Binary files and potentially other changes not applied. Please rebase and squash commits if necessary. |
||| _ Patch Compile Tests _ |
| +1 :green_heart: | mvninstall | 4m 51s | the patch passed |
| +1 :green_heart: | compile | 1m 21s | the patch passed |
| +1 :green_heart: | javac | 1m 21s | the patch passed |
| +1 :green_heart: | shadedjars | 9m 13s | patch has no errors when building our shaded downstream artifacts. |
| +1 :green_heart: | javadoc | 0m 41s | the patch passed |
||| _ Other Tests _ |
| -1 :x: | unit | 236m 57s | hbase-server in the patch failed. |
| | | 272m 28s | |
| 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-3721/12/artifact/yetus-jdk11-hadoop3-check/output/Dockerfile |
| GITHUB PR | https://github.com/apache/hbase/pull/3721 |
| Optional Tests | javac javadoc unit shadedjars compile |
| uname | Linux d8a0499f9a41 4.15.0-142-generic #146-Ubuntu SMP Tue Apr 13 01:11:19 UTC 2021 x86_64 x86_64 x86_64 GNU/Linux |
| Build tool | maven |
| Personality | dev-support/hbase-personality.sh |
| git revision | HBASE-26067 / c4d7d28911 |
| Default Java | AdoptOpenJDK-11.0.10+9 |
| unit | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-3721/12/artifact/yetus-jdk11-hadoop3-check/output/patch-unit-hbase-server.txt |
| Test Results | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-3721/12/testReport/ |
| Max. process+thread count | 2881 (vs. ulimit of 30000) |
| modules | C: hbase-server U: hbase-server |
| Console output | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-3721/12/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.
--
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.
To unsubscribe, e-mail: issues-unsubscribe@hbase.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [hbase] joshelser commented on a change in pull request #3721: HBASE-26326 CreateTableProcedure fails when FileBasedStoreFileTracker…
Posted by GitBox <gi...@apache.org>.
joshelser commented on a change in pull request #3721:
URL: https://github.com/apache/hbase/pull/3721#discussion_r727605083
##########
File path: hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/storefiletracker/StoreFileTracker.java
##########
@@ -75,13 +76,13 @@ void replace(Collection<StoreFileInfo> compactedFiles, Collection<StoreFileInfo>
StoreFileWriter createWriter(CreateStoreFileWriterParams params) throws IOException;
/**
- * Saves StoreFileTracker implementations specific configurations into the table descriptors.
+ * Adds StoreFileTracker implementations specific configurations into the table descriptor.
* <p/>
* This is used to avoid accidentally data loss when changing the cluster level store file tracker
* implementation, and also possible misconfiguration between master and region servers.
* <p/>
* See HBASE-26246 for more details.
* @param builder The table descriptor builder for the given table.
*/
- void persistConfiguration(TableDescriptorBuilder builder);
+ TableDescriptor updateWithTrackerConfigs(TableDescriptorBuilder builder);
Review comment:
Maybe `buildWithTrackerConfigs` if you're returning the `TableDescriptor` instead of the `TableDescriptorBuilder`. IMO, i'd just return the Builder back and let the caller `build()` when they're ready.
##########
File path: hbase-server/src/test/java/org/apache/hadoop/hbase/master/procedure/TestCreateTableProcedure.java
##########
@@ -105,6 +106,21 @@ public void testCreateWithTrackImpl() throws Exception {
assertEquals(trackerName, htd.getValue(TRACKER_IMPL));
}
+ @Test
+ public void testCreateWithFileBasedStoreTrackerImpl() throws Exception {
+ ProcedureExecutor<MasterProcedureEnv> procExec = getMasterProcedureExecutor();
+ procExec.getEnvironment().getMasterConfiguration().set(StoreFileTrackerFactory.TRACKER_IMPL,
+ StoreFileTrackerFactory.Trackers.FILE.name());
Review comment:
Does this need to be unset for other test methods in this class? Or, if tests are executed in parallel, maybe it would affects the other methods in this class? We set `forkCount` in surefire-plugin, but we don't set `parallel` (which I guess means we don't do parallel tests?). Genuinely not sure :)
##########
File path: hbase-server/src/test/java/org/apache/hadoop/hbase/master/procedure/TestCreateTableProcedure.java
##########
@@ -105,6 +106,21 @@ public void testCreateWithTrackImpl() throws Exception {
assertEquals(trackerName, htd.getValue(TRACKER_IMPL));
}
+ @Test
+ public void testCreateWithFileBasedStoreTrackerImpl() throws Exception {
+ ProcedureExecutor<MasterProcedureEnv> procExec = getMasterProcedureExecutor();
+ procExec.getEnvironment().getMasterConfiguration().set(StoreFileTrackerFactory.TRACKER_IMPL,
+ StoreFileTrackerFactory.Trackers.FILE.name());
Review comment:
ok, cool. I didn't _think_ we ran test methods concurrently, but was stymied when I tried to quickly figure that out from the pom.xml :P
--
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.
To unsubscribe, e-mail: issues-unsubscribe@hbase.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [hbase] Apache-HBase commented on pull request #3721: HBASE-26326 CreateTableProcedure fails when FileBasedStoreFileTracker…
Posted by GitBox <gi...@apache.org>.
Apache-HBase commented on pull request #3721:
URL: https://github.com/apache/hbase/pull/3721#issuecomment-938754865
:confetti_ball: **+1 overall**
| Vote | Subsystem | Runtime | Comment |
|:----:|----------:|--------:|:--------|
| +0 :ok: | reexec | 0m 38s | Docker mode activated. |
||| _ Prechecks _ |
| +1 :green_heart: | dupname | 0m 0s | No case conflicting files found. |
| +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. |
||| _ HBASE-26067 Compile Tests _ |
| +1 :green_heart: | mvninstall | 4m 22s | HBASE-26067 passed |
| +1 :green_heart: | compile | 3m 50s | HBASE-26067 passed |
| +1 :green_heart: | checkstyle | 1m 14s | HBASE-26067 passed |
| +1 :green_heart: | spotbugs | 2m 32s | HBASE-26067 passed |
| -0 :warning: | patch | 2m 40s | Used diff version of patch file. Binary files and potentially other changes not applied. Please rebase and squash commits if necessary. |
||| _ Patch Compile Tests _ |
| +1 :green_heart: | mvninstall | 4m 21s | the patch passed |
| +1 :green_heart: | compile | 3m 46s | the patch passed |
| +1 :green_heart: | javac | 3m 46s | the patch passed |
| +1 :green_heart: | checkstyle | 1m 10s | the patch passed |
| +1 :green_heart: | whitespace | 0m 0s | The patch has no whitespace issues. |
| +1 :green_heart: | hadoopcheck | 21m 15s | Patch does not cause any errors with Hadoop 3.1.2 3.2.1 3.3.0. |
| +1 :green_heart: | spotbugs | 2m 37s | the patch passed |
||| _ Other Tests _ |
| +1 :green_heart: | asflicense | 0m 13s | The patch does not generate ASF License warnings. |
| | | 54m 58s | |
| 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-3721/12/artifact/yetus-general-check/output/Dockerfile |
| GITHUB PR | https://github.com/apache/hbase/pull/3721 |
| Optional Tests | dupname asflicense javac spotbugs hadoopcheck hbaseanti checkstyle compile |
| uname | Linux 3608d83ecbe5 4.15.0-156-generic #163-Ubuntu SMP Thu Aug 19 23:31:58 UTC 2021 x86_64 x86_64 x86_64 GNU/Linux |
| Build tool | maven |
| Personality | dev-support/hbase-personality.sh |
| git revision | HBASE-26067 / c4d7d28911 |
| Default Java | AdoptOpenJDK-1.8.0_282-b08 |
| Max. process+thread count | 96 (vs. ulimit of 30000) |
| modules | C: hbase-server U: hbase-server |
| Console output | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-3721/12/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.
--
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.
To unsubscribe, e-mail: issues-unsubscribe@hbase.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org