You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@hbase.apache.org by GitBox <gi...@apache.org> on 2020/08/26 09:02:00 UTC
[GitHub] [hbase] Apache9 opened a new pull request #2317: HBASE-24949 Optimize FSTableDescriptors.get to not always go to fs wh…
Apache9 opened a new pull request #2317:
URL: https://github.com/apache/hbase/pull/2317
…en cache miss
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [hbase] Apache9 commented on a change in pull request #2317: HBASE-24949 Optimize FSTableDescriptors.get to not always go to fs wh…
Posted by GitBox <gi...@apache.org>.
Apache9 commented on a change in pull request #2317:
URL: https://github.com/apache/hbase/pull/2317#discussion_r478318570
##########
File path: hbase-server/src/main/java/org/apache/hadoop/hbase/util/FSTableDescriptors.java
##########
@@ -290,35 +272,43 @@ public TableDescriptor get(final TableName tablename)
* @see #get(org.apache.hadoop.hbase.TableName)
*/
@Override
- public Map<String, TableDescriptor> getByNamespace(String name)
- throws IOException {
+ public Map<String, TableDescriptor> getByNamespace(String name) throws IOException {
Map<String, TableDescriptor> htds = new TreeMap<>();
List<Path> tableDirs =
- FSUtils.getLocalTableDirs(fs, CommonFSUtils.getNamespaceDir(rootdir, name));
- for (Path d: tableDirs) {
- TableDescriptor htd = null;
- try {
- htd = get(CommonFSUtils.getTableName(d));
- } catch (FileNotFoundException fnfe) {
- // inability of retrieving one HTD shouldn't stop getting the remaining
- LOG.warn("Trouble retrieving htd", fnfe);
+ FSUtils.getLocalTableDirs(fs, CommonFSUtils.getNamespaceDir(rootdir, name));
+ for (Path d : tableDirs) {
+ TableDescriptor htd = get(CommonFSUtils.getTableName(d));
+ if (htd == null) {
+ continue;
}
- if (htd == null) continue;
htds.put(CommonFSUtils.getTableName(d).getNameAsString(), htd);
}
return htds;
}
- /**
- * Adds (or updates) the table descriptor to the FileSystem
- * and updates the local cache with it.
- */
@Override
- public void update(TableDescriptor htd) throws IOException {
+ public void update(TableDescriptor td, boolean cacheOnly) throws IOException {
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [hbase] infraio commented on a change in pull request #2317: HBASE-24949 Optimize FSTableDescriptors.get to not always go to fs wh…
Posted by GitBox <gi...@apache.org>.
infraio commented on a change in pull request #2317:
URL: https://github.com/apache/hbase/pull/2317#discussion_r478432991
##########
File path: hbase-server/src/main/java/org/apache/hadoop/hbase/util/FSTableDescriptors.java
##########
@@ -290,35 +272,48 @@ public TableDescriptor get(final TableName tablename)
* @see #get(org.apache.hadoop.hbase.TableName)
*/
@Override
- public Map<String, TableDescriptor> getByNamespace(String name)
- throws IOException {
+ public Map<String, TableDescriptor> getByNamespace(String name) throws IOException {
Map<String, TableDescriptor> htds = new TreeMap<>();
List<Path> tableDirs =
- FSUtils.getLocalTableDirs(fs, CommonFSUtils.getNamespaceDir(rootdir, name));
- for (Path d: tableDirs) {
- TableDescriptor htd = null;
- try {
- htd = get(CommonFSUtils.getTableName(d));
- } catch (FileNotFoundException fnfe) {
- // inability of retrieving one HTD shouldn't stop getting the remaining
- LOG.warn("Trouble retrieving htd", fnfe);
+ FSUtils.getLocalTableDirs(fs, CommonFSUtils.getNamespaceDir(rootdir, name));
+ for (Path d : tableDirs) {
+ TableDescriptor htd = get(CommonFSUtils.getTableName(d));
+ if (htd == null) {
+ continue;
}
- if (htd == null) continue;
htds.put(CommonFSUtils.getTableName(d).getNameAsString(), htd);
}
return htds;
}
- /**
- * Adds (or updates) the table descriptor to the FileSystem
- * and updates the local cache with it.
- */
@Override
- public void update(TableDescriptor htd) throws IOException {
+ public void update(TableDescriptor td, boolean cacheOnly) throws IOException {
+ // TODO: in fact this method will only be called at master side, so fsreadonly and usecache will
+ // always be true. In general, we'd better have a ReadOnlyFSTableDesciptors for HRegionServer
+ // but now, HMaster extends HRegionServer, so unless making use of generic, we can not have
+ // different implementations for HMaster and HRegionServer. Revisit this when we make HMaster
+ // not extend HRegionServer in the future.
if (fsreadonly) {
- throw new NotImplementedException("Cannot add a table descriptor - in read only mode");
+ throw new UnsupportedOperationException("Cannot add a table descriptor - in read only mode");
+ }
+ if (!cacheOnly) {
+ updateTableDesciptor(td);
+ }
+ if (usecache) {
+ this.cache.put(td.getTableName(), td);
}
- updateTableDescriptor(htd);
+ }
+
+ @VisibleForTesting
+ Path updateTableDesciptor(TableDescriptor td) throws IOException {
Review comment:
updateTableDesciptor => updateTableDescriptor
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [hbase] virajjasani commented on a change in pull request #2317: HBASE-24949 Optimize FSTableDescriptors.get to not always go to fs wh…
Posted by GitBox <gi...@apache.org>.
virajjasani commented on a change in pull request #2317:
URL: https://github.com/apache/hbase/pull/2317#discussion_r477170362
##########
File path: hbase-server/src/main/java/org/apache/hadoop/hbase/util/FSTableDescriptors.java
##########
@@ -196,57 +197,47 @@ private static TableDescriptorBuilder createMetaTableDescriptorBuilder(final Con
.setPriority(Coprocessor.PRIORITY_SYSTEM).build());
}
- @Override
- public void setCacheOn() throws IOException {
- this.cache.clear();
- this.usecache = true;
- }
-
- @Override
- public void setCacheOff() throws IOException {
- this.usecache = false;
- this.cache.clear();
- }
-
@VisibleForTesting
public boolean isUsecache() {
return this.usecache;
}
/**
* Get the current table descriptor for the given table, or null if none exists.
- *
- * Uses a local cache of the descriptor but still checks the filesystem on each call
- * to see if a newer file has been created since the cached one was read.
+ * <p/>
+ * Uses a local cache of the descriptor but still checks the filesystem on each call if
+ * {@link #fsvisited} is not {@code true}, i.e, we haven't done a full scan yet, to see if a newer
+ * file has been created since the cached one was read.
*/
@Override
@Nullable
- public TableDescriptor get(final TableName tablename)
- throws IOException {
+ public TableDescriptor get(TableName tableName) throws IOException {
invocations++;
if (usecache) {
// Look in cache of descriptors.
- TableDescriptor cachedtdm = this.cache.get(tablename);
+ TableDescriptor cachedtdm = this.cache.get(tableName);
if (cachedtdm != null) {
cachehits++;
return cachedtdm;
}
+ // we do not need to go to fs any more
+ if (fsvisited) {
+ return null;
+ }
}
TableDescriptor tdmt = null;
try {
- tdmt = getTableDescriptorFromFs(fs, rootdir, tablename);
+ tdmt = getTableDescriptorFromFs(fs, rootdir, tableName);
} catch (NullPointerException e) {
- LOG.debug("Exception during readTableDecriptor. Current table name = "
- + tablename, e);
+ LOG.debug("Exception during readTableDecriptor. Current table name = " + tableName, e);
} catch (TableInfoMissingException e) {
// ignore. This is regular operation
} catch (IOException ioe) {
Review comment:
nit: `catch (NullPointerException | IOException e)`
##########
File path: hbase-server/src/main/java/org/apache/hadoop/hbase/util/FSTableDescriptors.java
##########
@@ -279,8 +270,8 @@ public TableDescriptor get(final TableName tablename)
} else {
Review comment:
nit: In above if condition, `continue` is no longer required.
##########
File path: hbase-server/src/main/java/org/apache/hadoop/hbase/util/FSTableDescriptors.java
##########
@@ -125,11 +125,11 @@ public static void tryUpdateMetaTableDescriptor(Configuration conf) throws IOExc
CommonFSUtils.getRootDir(conf), null);
}
- public static void tryUpdateMetaTableDescriptor(Configuration conf, FileSystem fs, Path rootdir,
+ public static TableDescriptor tryUpdateMetaTableDescriptor(Configuration conf, FileSystem fs, Path rootdir,
Review comment:
nit: method name can represent returning TableDescriptor, maybe something like: `updateAndGetMetaTableDesc` ?
##########
File path: hbase-server/src/main/java/org/apache/hadoop/hbase/util/FSTableDescriptors.java
##########
@@ -79,7 +79,7 @@
private final FileSystem fs;
private final Path rootdir;
private final boolean fsreadonly;
- private volatile boolean usecache;
+ private final boolean usecache;
Review comment:
Now that we only have constructor to set this, +1
##########
File path: hbase-server/src/main/java/org/apache/hadoop/hbase/TableDescriptors.java
##########
@@ -31,7 +31,7 @@
/**
* @return TableDescriptor for tablename
*/
- TableDescriptor get(final TableName tableName) throws IOException;
+ TableDescriptor get(TableName tableName) throws IOException;
Review comment:
Is this change needed? Do we follow any convention to not provide `final` to argument? Just asking as I am not aware of it.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [hbase] Apache9 merged pull request #2317: HBASE-24949 Optimize FSTableDescriptors.get to not always go to fs wh…
Posted by GitBox <gi...@apache.org>.
Apache9 merged pull request #2317:
URL: https://github.com/apache/hbase/pull/2317
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [hbase] Apache9 commented on a change in pull request #2317: HBASE-24949 Optimize FSTableDescriptors.get to not always go to fs wh…
Posted by GitBox <gi...@apache.org>.
Apache9 commented on a change in pull request #2317:
URL: https://github.com/apache/hbase/pull/2317#discussion_r477175910
##########
File path: hbase-server/src/main/java/org/apache/hadoop/hbase/TableDescriptors.java
##########
@@ -31,7 +31,7 @@
/**
* @return TableDescriptor for tablename
*/
- TableDescriptor get(final TableName tableName) throws IOException;
+ TableDescriptor get(TableName tableName) throws IOException;
Review comment:
In the interface the final is useless, you are free to not use final when actually implementing the method, and there is no difference for callers. So just remove it.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [hbase] Apache-HBase commented on pull request #2317: HBASE-24949 Optimize FSTableDescriptors.get to not always go to fs wh…
Posted by GitBox <gi...@apache.org>.
Apache-HBase commented on pull request #2317:
URL: https://github.com/apache/hbase/pull/2317#issuecomment-682303918
: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. |
||| _ master Compile Tests _ |
| +1 :green_heart: | mvninstall | 4m 6s | master passed |
| +1 :green_heart: | checkstyle | 1m 13s | master passed |
| +1 :green_heart: | spotbugs | 2m 5s | master passed |
||| _ Patch Compile Tests _ |
| +1 :green_heart: | mvninstall | 3m 23s | the patch passed |
| +1 :green_heart: | checkstyle | 1m 4s | hbase-server: The patch generated 0 new + 168 unchanged - 11 fixed = 168 total (was 179) |
| +1 :green_heart: | whitespace | 0m 0s | The patch has no whitespace issues. |
| +1 :green_heart: | hadoopcheck | 11m 51s | Patch does not cause any errors with Hadoop 3.1.2 3.2.1. |
| +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. |
| | | 34m 0s | |
| Subsystem | Report/Notes |
|----------:|:-------------|
| Docker | Client=19.03.12 Server=19.03.12 base: https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2317/7/artifact/yetus-general-check/output/Dockerfile |
| GITHUB PR | https://github.com/apache/hbase/pull/2317 |
| Optional Tests | dupname asflicense spotbugs hadoopcheck hbaseanti checkstyle |
| uname | Linux d9167e726189 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 | master / 047e0618d2 |
| Max. process+thread count | 94 (vs. ulimit of 12500) |
| modules | C: hbase-server U: hbase-server |
| Console output | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2317/7/console |
| versions | git=2.17.1 maven=(cecedd343002696d0abb50b32b541b8a6ba2883f) spotbugs=3.1.12 |
| Powered by | Apache Yetus 0.11.1 https://yetus.apache.org |
This message was automatically generated.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [hbase] Apache-HBase commented on pull request #2317: HBASE-24949 Optimize FSTableDescriptors.get to not always go to fs wh…
Posted by GitBox <gi...@apache.org>.
Apache-HBase commented on pull request #2317:
URL: https://github.com/apache/hbase/pull/2317#issuecomment-681879020
:confetti_ball: **+1 overall**
| Vote | Subsystem | Runtime | Comment |
|:----:|----------:|--------:|:--------|
| +0 :ok: | reexec | 0m 29s | 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. |
||| _ master Compile Tests _ |
| +1 :green_heart: | mvninstall | 3m 57s | master passed |
| +1 :green_heart: | checkstyle | 1m 12s | master passed |
| +1 :green_heart: | spotbugs | 2m 3s | master passed |
||| _ Patch Compile Tests _ |
| +1 :green_heart: | mvninstall | 3m 33s | the patch passed |
| +1 :green_heart: | checkstyle | 1m 13s | hbase-server: The patch generated 0 new + 168 unchanged - 11 fixed = 168 total (was 179) |
| +1 :green_heart: | whitespace | 0m 0s | The patch has no whitespace issues. |
| +1 :green_heart: | hadoopcheck | 11m 49s | Patch does not cause any errors with Hadoop 3.1.2 3.2.1. |
| +1 :green_heart: | spotbugs | 2m 16s | the patch passed |
||| _ Other Tests _ |
| +1 :green_heart: | asflicense | 0m 16s | The patch does not generate ASF License warnings. |
| | | 34m 18s | |
| Subsystem | Report/Notes |
|----------:|:-------------|
| Docker | Client=19.03.12 Server=19.03.12 base: https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2317/5/artifact/yetus-general-check/output/Dockerfile |
| GITHUB PR | https://github.com/apache/hbase/pull/2317 |
| Optional Tests | dupname asflicense spotbugs hadoopcheck hbaseanti checkstyle |
| uname | Linux dc4142d45826 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 | master / 047e0618d2 |
| Max. process+thread count | 94 (vs. ulimit of 12500) |
| modules | C: hbase-server U: hbase-server |
| Console output | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2317/5/console |
| versions | git=2.17.1 maven=(cecedd343002696d0abb50b32b541b8a6ba2883f) spotbugs=3.1.12 |
| Powered by | Apache Yetus 0.11.1 https://yetus.apache.org |
This message was automatically generated.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [hbase] Apache9 commented on pull request #2317: HBASE-24949 Optimize FSTableDescriptors.get to not always go to fs wh…
Posted by GitBox <gi...@apache.org>.
Apache9 commented on pull request #2317:
URL: https://github.com/apache/hbase/pull/2317#issuecomment-680805197
There are some problems on updating the cache. I assume the old logic just worked accidentally, because we always go to the fs if no entires in cache...
Let me think how to fix 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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [hbase] Apache-HBase commented on pull request #2317: HBASE-24949 Optimize FSTableDescriptors.get to not always go to fs wh…
Posted by GitBox <gi...@apache.org>.
Apache-HBase commented on pull request #2317:
URL: https://github.com/apache/hbase/pull/2317#issuecomment-682001089
:confetti_ball: **+1 overall**
| Vote | Subsystem | Runtime | Comment |
|:----:|----------:|--------:|:--------|
| +0 :ok: | reexec | 0m 31s | Docker mode activated. |
||| _ Prechecks _ |
| +1 :green_heart: | dupname | 0m 0s | No case conflicting files found. |
| +1 :green_heart: | hbaseanti | 0m 0s | Patch does not have any anti-patterns. |
| +1 :green_heart: | @author | 0m 0s | The patch does not contain any @author tags. |
||| _ master Compile Tests _ |
| +1 :green_heart: | mvninstall | 3m 32s | master passed |
| +1 :green_heart: | checkstyle | 1m 9s | master passed |
| +1 :green_heart: | spotbugs | 2m 4s | master passed |
||| _ Patch Compile Tests _ |
| +1 :green_heart: | mvninstall | 3m 32s | the patch passed |
| +1 :green_heart: | checkstyle | 1m 7s | hbase-server: The patch generated 0 new + 168 unchanged - 11 fixed = 168 total (was 179) |
| +1 :green_heart: | whitespace | 0m 0s | The patch has no whitespace issues. |
| +1 :green_heart: | hadoopcheck | 11m 36s | Patch does not cause any errors with Hadoop 3.1.2 3.2.1. |
| +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. |
| | | 33m 22s | |
| Subsystem | Report/Notes |
|----------:|:-------------|
| Docker | Client=19.03.12 Server=19.03.12 base: https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2317/6/artifact/yetus-general-check/output/Dockerfile |
| GITHUB PR | https://github.com/apache/hbase/pull/2317 |
| Optional Tests | dupname asflicense spotbugs hadoopcheck hbaseanti checkstyle |
| uname | Linux 5e960051b26a 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 | master / 047e0618d2 |
| Max. process+thread count | 94 (vs. ulimit of 12500) |
| modules | C: hbase-server U: hbase-server |
| Console output | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2317/6/console |
| versions | git=2.17.1 maven=(cecedd343002696d0abb50b32b541b8a6ba2883f) spotbugs=3.1.12 |
| Powered by | Apache Yetus 0.11.1 https://yetus.apache.org |
This message was automatically generated.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [hbase] Apache-HBase commented on pull request #2317: HBASE-24949 Optimize FSTableDescriptors.get to not always go to fs wh…
Posted by GitBox <gi...@apache.org>.
Apache-HBase commented on pull request #2317:
URL: https://github.com/apache/hbase/pull/2317#issuecomment-680787433
:confetti_ball: **+1 overall**
| Vote | Subsystem | Runtime | Comment |
|:----:|----------:|--------:|:--------|
| +0 :ok: | reexec | 0m 39s | 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. |
||| _ master Compile Tests _ |
| +1 :green_heart: | mvninstall | 5m 5s | master passed |
| +1 :green_heart: | checkstyle | 1m 27s | master passed |
| +1 :green_heart: | spotbugs | 2m 41s | master passed |
||| _ Patch Compile Tests _ |
| +1 :green_heart: | mvninstall | 4m 39s | the patch passed |
| -0 :warning: | checkstyle | 1m 31s | hbase-server: The patch generated 1 new + 168 unchanged - 1 fixed = 169 total (was 169) |
| +1 :green_heart: | whitespace | 0m 0s | The patch has no whitespace issues. |
| +1 :green_heart: | hadoopcheck | 15m 15s | Patch does not cause any errors with Hadoop 3.1.2 3.2.1. |
| +1 :green_heart: | spotbugs | 2m 48s | the patch passed |
||| _ Other Tests _ |
| +1 :green_heart: | asflicense | 0m 14s | The patch does not generate ASF License warnings. |
| | | 43m 9s | |
| Subsystem | Report/Notes |
|----------:|:-------------|
| Docker | Client=19.03.12 Server=19.03.12 base: https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2317/1/artifact/yetus-general-check/output/Dockerfile |
| GITHUB PR | https://github.com/apache/hbase/pull/2317 |
| Optional Tests | dupname asflicense spotbugs hadoopcheck hbaseanti checkstyle |
| uname | Linux 5efdb7fd46b0 4.15.0-58-generic #64-Ubuntu SMP Tue Aug 6 11:12:41 UTC 2019 x86_64 x86_64 x86_64 GNU/Linux |
| Build tool | maven |
| Personality | dev-support/hbase-personality.sh |
| git revision | master / 01cf60067c |
| checkstyle | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2317/1/artifact/yetus-general-check/output/diff-checkstyle-hbase-server.txt |
| Max. process+thread count | 94 (vs. ulimit of 12500) |
| modules | C: hbase-server U: hbase-server |
| Console output | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2317/1/console |
| versions | git=2.17.1 maven=(cecedd343002696d0abb50b32b541b8a6ba2883f) spotbugs=3.1.12 |
| Powered by | Apache Yetus 0.11.1 https://yetus.apache.org |
This message was automatically generated.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [hbase] Apache-HBase commented on pull request #2317: HBASE-24949 Optimize FSTableDescriptors.get to not always go to fs wh…
Posted by GitBox <gi...@apache.org>.
Apache-HBase commented on pull request #2317:
URL: https://github.com/apache/hbase/pull/2317#issuecomment-680952145
:confetti_ball: **+1 overall**
| Vote | Subsystem | Runtime | Comment |
|:----:|----------:|--------:|:--------|
| +0 :ok: | reexec | 0m 29s | 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. |
||| _ master Compile Tests _ |
| +1 :green_heart: | mvninstall | 4m 3s | master passed |
| +1 :green_heart: | checkstyle | 1m 13s | master passed |
| +1 :green_heart: | spotbugs | 2m 13s | master passed |
||| _ Patch Compile Tests _ |
| +1 :green_heart: | mvninstall | 3m 43s | the patch passed |
| -0 :warning: | checkstyle | 1m 11s | hbase-server: The patch generated 1 new + 168 unchanged - 11 fixed = 169 total (was 179) |
| +1 :green_heart: | whitespace | 0m 0s | The patch has no whitespace issues. |
| +1 :green_heart: | hadoopcheck | 12m 11s | Patch does not cause any errors with Hadoop 3.1.2 3.2.1. |
| +1 :green_heart: | spotbugs | 2m 20s | the patch passed |
||| _ Other Tests _ |
| +1 :green_heart: | asflicense | 0m 16s | The patch does not generate ASF License warnings. |
| | | 35m 22s | |
| Subsystem | Report/Notes |
|----------:|:-------------|
| Docker | Client=19.03.12 Server=19.03.12 base: https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2317/2/artifact/yetus-general-check/output/Dockerfile |
| GITHUB PR | https://github.com/apache/hbase/pull/2317 |
| Optional Tests | dupname asflicense spotbugs hadoopcheck hbaseanti checkstyle |
| uname | Linux a1929dda06c9 4.15.0-60-generic #67-Ubuntu SMP Thu Aug 22 16:55:30 UTC 2019 x86_64 x86_64 x86_64 GNU/Linux |
| Build tool | maven |
| Personality | dev-support/hbase-personality.sh |
| git revision | master / 01cf60067c |
| checkstyle | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2317/2/artifact/yetus-general-check/output/diff-checkstyle-hbase-server.txt |
| Max. process+thread count | 94 (vs. ulimit of 12500) |
| modules | C: hbase-server U: hbase-server |
| Console output | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2317/2/console |
| versions | git=2.17.1 maven=(cecedd343002696d0abb50b32b541b8a6ba2883f) spotbugs=3.1.12 |
| Powered by | Apache Yetus 0.11.1 https://yetus.apache.org |
This message was automatically generated.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [hbase] Apache-HBase commented on pull request #2317: HBASE-24949 Optimize FSTableDescriptors.get to not always go to fs wh…
Posted by GitBox <gi...@apache.org>.
Apache-HBase commented on pull request #2317:
URL: https://github.com/apache/hbase/pull/2317#issuecomment-683249148
:confetti_ball: **+1 overall**
| Vote | Subsystem | Runtime | Comment |
|:----:|----------:|--------:|:--------|
| +0 :ok: | reexec | 1m 3s | Docker mode activated. |
| -0 :warning: | yetus | 0m 3s | Unprocessed flag(s): --brief-report-file --spotbugs-strict-precheck --whitespace-eol-ignore-list --whitespace-tabs-ignore-list --quick-hadoopcheck |
||| _ Prechecks _ |
||| _ master Compile Tests _ |
| +1 :green_heart: | mvninstall | 4m 35s | master passed |
| +1 :green_heart: | compile | 1m 10s | master passed |
| +1 :green_heart: | shadedjars | 7m 19s | branch has no errors when building our shaded downstream artifacts. |
| -0 :warning: | javadoc | 0m 41s | hbase-server in master failed. |
||| _ Patch Compile Tests _ |
| +1 :green_heart: | mvninstall | 4m 35s | the patch passed |
| +1 :green_heart: | compile | 1m 10s | the patch passed |
| +1 :green_heart: | javac | 1m 10s | the patch passed |
| +1 :green_heart: | shadedjars | 7m 14s | patch has no errors when building our shaded downstream artifacts. |
| -0 :warning: | javadoc | 0m 40s | hbase-server in the patch failed. |
||| _ Other Tests _ |
| +1 :green_heart: | unit | 195m 22s | hbase-server in the patch passed. |
| | | 225m 41s | |
| Subsystem | Report/Notes |
|----------:|:-------------|
| Docker | Client=19.03.12 Server=19.03.12 base: https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2317/8/artifact/yetus-jdk11-hadoop3-check/output/Dockerfile |
| GITHUB PR | https://github.com/apache/hbase/pull/2317 |
| Optional Tests | javac javadoc unit shadedjars compile |
| uname | Linux 7c92e32ba716 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 | master / 7909e29de5 |
| Default Java | 2020-01-14 |
| javadoc | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2317/8/artifact/yetus-jdk11-hadoop3-check/output/branch-javadoc-hbase-server.txt |
| javadoc | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2317/8/artifact/yetus-jdk11-hadoop3-check/output/patch-javadoc-hbase-server.txt |
| Test Results | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2317/8/testReport/ |
| Max. process+thread count | 3548 (vs. ulimit of 12500) |
| modules | C: hbase-server U: hbase-server |
| Console output | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2317/8/console |
| versions | git=2.17.1 maven=(cecedd343002696d0abb50b32b541b8a6ba2883f) |
| Powered by | Apache Yetus 0.11.1 https://yetus.apache.org |
This message was automatically generated.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [hbase] Apache-HBase commented on pull request #2317: HBASE-24949 Optimize FSTableDescriptors.get to not always go to fs wh…
Posted by GitBox <gi...@apache.org>.
Apache-HBase commented on pull request #2317:
URL: https://github.com/apache/hbase/pull/2317#issuecomment-681813527
:broken_heart: **-1 overall**
| Vote | Subsystem | Runtime | Comment |
|:----:|----------:|--------:|:--------|
| +0 :ok: | reexec | 1m 41s | Docker mode activated. |
| -0 :warning: | yetus | 0m 3s | Unprocessed flag(s): --brief-report-file --spotbugs-strict-precheck --whitespace-eol-ignore-list --whitespace-tabs-ignore-list --quick-hadoopcheck |
||| _ Prechecks _ |
||| _ master Compile Tests _ |
| +1 :green_heart: | mvninstall | 4m 54s | master passed |
| +1 :green_heart: | compile | 1m 18s | master passed |
| +1 :green_heart: | shadedjars | 7m 37s | branch has no errors when building our shaded downstream artifacts. |
| -0 :warning: | javadoc | 0m 46s | hbase-server in master failed. |
||| _ Patch Compile Tests _ |
| +1 :green_heart: | mvninstall | 4m 41s | the patch passed |
| +1 :green_heart: | compile | 1m 27s | the patch passed |
| +1 :green_heart: | javac | 1m 27s | the patch passed |
| +1 :green_heart: | shadedjars | 8m 10s | patch has no errors when building our shaded downstream artifacts. |
| -0 :warning: | javadoc | 0m 45s | hbase-server in the patch failed. |
||| _ Other Tests _ |
| -1 :x: | unit | 10m 38s | hbase-server in the patch failed. |
| | | 43m 16s | |
| Subsystem | Report/Notes |
|----------:|:-------------|
| Docker | Client=19.03.12 Server=19.03.12 base: https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2317/4/artifact/yetus-jdk11-hadoop3-check/output/Dockerfile |
| GITHUB PR | https://github.com/apache/hbase/pull/2317 |
| Optional Tests | javac javadoc unit shadedjars compile |
| uname | Linux ca7b7d2b4003 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 | master / 047e0618d2 |
| Default Java | 2020-01-14 |
| javadoc | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2317/4/artifact/yetus-jdk11-hadoop3-check/output/branch-javadoc-hbase-server.txt |
| javadoc | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2317/4/artifact/yetus-jdk11-hadoop3-check/output/patch-javadoc-hbase-server.txt |
| unit | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2317/4/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-2317/4/testReport/ |
| Max. process+thread count | 434 (vs. ulimit of 12500) |
| modules | C: hbase-server U: hbase-server |
| Console output | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2317/4/console |
| versions | git=2.17.1 maven=(cecedd343002696d0abb50b32b541b8a6ba2883f) |
| Powered by | Apache Yetus 0.11.1 https://yetus.apache.org |
This message was automatically generated.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [hbase] Apache-HBase commented on pull request #2317: HBASE-24949 Optimize FSTableDescriptors.get to not always go to fs wh…
Posted by GitBox <gi...@apache.org>.
Apache-HBase commented on pull request #2317:
URL: https://github.com/apache/hbase/pull/2317#issuecomment-683249545
:confetti_ball: **+1 overall**
| Vote | Subsystem | Runtime | Comment |
|:----:|----------:|--------:|:--------|
| +0 :ok: | reexec | 1m 16s | Docker mode activated. |
| -0 :warning: | yetus | 0m 3s | Unprocessed flag(s): --brief-report-file --spotbugs-strict-precheck --whitespace-eol-ignore-list --whitespace-tabs-ignore-list --quick-hadoopcheck |
||| _ Prechecks _ |
||| _ master Compile Tests _ |
| +1 :green_heart: | mvninstall | 4m 18s | master passed |
| +1 :green_heart: | compile | 0m 59s | master passed |
| +1 :green_heart: | shadedjars | 7m 7s | branch has no errors when building our shaded downstream artifacts. |
| +1 :green_heart: | javadoc | 0m 38s | master passed |
||| _ Patch Compile Tests _ |
| +1 :green_heart: | mvninstall | 3m 47s | the patch passed |
| +1 :green_heart: | compile | 0m 58s | the patch passed |
| +1 :green_heart: | javac | 0m 58s | the patch passed |
| +1 :green_heart: | shadedjars | 7m 11s | 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 | 202m 21s | hbase-server in the patch passed. |
| | | 230m 18s | |
| Subsystem | Report/Notes |
|----------:|:-------------|
| Docker | Client=19.03.12 Server=19.03.12 base: https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2317/8/artifact/yetus-jdk8-hadoop3-check/output/Dockerfile |
| GITHUB PR | https://github.com/apache/hbase/pull/2317 |
| Optional Tests | javac javadoc unit shadedjars compile |
| uname | Linux a20748d94ab7 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 | master / 7909e29de5 |
| Default Java | 1.8.0_232 |
| Test Results | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2317/8/testReport/ |
| Max. process+thread count | 3069 (vs. ulimit of 12500) |
| modules | C: hbase-server U: hbase-server |
| Console output | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2317/8/console |
| versions | git=2.17.1 maven=(cecedd343002696d0abb50b32b541b8a6ba2883f) |
| Powered by | Apache Yetus 0.11.1 https://yetus.apache.org |
This message was automatically generated.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [hbase] Apache-HBase commented on pull request #2317: HBASE-24949 Optimize FSTableDescriptors.get to not always go to fs wh…
Posted by GitBox <gi...@apache.org>.
Apache-HBase commented on pull request #2317:
URL: https://github.com/apache/hbase/pull/2317#issuecomment-681806400
:confetti_ball: **+1 overall**
| Vote | Subsystem | Runtime | Comment |
|:----:|----------:|--------:|:--------|
| +0 :ok: | reexec | 1m 36s | 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. |
||| _ master Compile Tests _ |
| +1 :green_heart: | mvninstall | 4m 16s | master passed |
| +1 :green_heart: | checkstyle | 1m 15s | master passed |
| +1 :green_heart: | spotbugs | 2m 38s | master passed |
||| _ Patch Compile Tests _ |
| +1 :green_heart: | mvninstall | 4m 8s | the patch passed |
| +1 :green_heart: | checkstyle | 1m 26s | hbase-server: The patch generated 0 new + 168 unchanged - 11 fixed = 168 total (was 179) |
| +1 :green_heart: | whitespace | 0m 0s | The patch has no whitespace issues. |
| +1 :green_heart: | hadoopcheck | 12m 46s | Patch does not cause any errors with Hadoop 3.1.2 3.2.1. |
| +1 :green_heart: | spotbugs | 2m 18s | the patch passed |
||| _ Other Tests _ |
| +1 :green_heart: | asflicense | 0m 12s | The patch does not generate ASF License warnings. |
| | | 38m 33s | |
| Subsystem | Report/Notes |
|----------:|:-------------|
| Docker | Client=19.03.12 Server=19.03.12 base: https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2317/4/artifact/yetus-general-check/output/Dockerfile |
| GITHUB PR | https://github.com/apache/hbase/pull/2317 |
| Optional Tests | dupname asflicense spotbugs hadoopcheck hbaseanti checkstyle |
| uname | Linux a404e114d915 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 | master / 047e0618d2 |
| Max. process+thread count | 84 (vs. ulimit of 12500) |
| modules | C: hbase-server U: hbase-server |
| Console output | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2317/4/console |
| versions | git=2.17.1 maven=(cecedd343002696d0abb50b32b541b8a6ba2883f) spotbugs=3.1.12 |
| Powered by | Apache Yetus 0.11.1 https://yetus.apache.org |
This message was automatically generated.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [hbase] Apache9 commented on a change in pull request #2317: HBASE-24949 Optimize FSTableDescriptors.get to not always go to fs wh…
Posted by GitBox <gi...@apache.org>.
Apache9 commented on a change in pull request #2317:
URL: https://github.com/apache/hbase/pull/2317#discussion_r478162122
##########
File path: hbase-server/src/main/java/org/apache/hadoop/hbase/util/FSTableDescriptors.java
##########
@@ -290,35 +272,43 @@ public TableDescriptor get(final TableName tablename)
* @see #get(org.apache.hadoop.hbase.TableName)
*/
@Override
- public Map<String, TableDescriptor> getByNamespace(String name)
- throws IOException {
+ public Map<String, TableDescriptor> getByNamespace(String name) throws IOException {
Map<String, TableDescriptor> htds = new TreeMap<>();
List<Path> tableDirs =
- FSUtils.getLocalTableDirs(fs, CommonFSUtils.getNamespaceDir(rootdir, name));
- for (Path d: tableDirs) {
- TableDescriptor htd = null;
- try {
- htd = get(CommonFSUtils.getTableName(d));
- } catch (FileNotFoundException fnfe) {
- // inability of retrieving one HTD shouldn't stop getting the remaining
- LOG.warn("Trouble retrieving htd", fnfe);
+ FSUtils.getLocalTableDirs(fs, CommonFSUtils.getNamespaceDir(rootdir, name));
+ for (Path d : tableDirs) {
+ TableDescriptor htd = get(CommonFSUtils.getTableName(d));
+ if (htd == null) {
+ continue;
}
- if (htd == null) continue;
htds.put(CommonFSUtils.getTableName(d).getNameAsString(), htd);
}
return htds;
}
- /**
- * Adds (or updates) the table descriptor to the FileSystem
- * and updates the local cache with it.
- */
@Override
- public void update(TableDescriptor htd) throws IOException {
+ public void update(TableDescriptor td, boolean cacheOnly) throws IOException {
Review comment:
In fact this will only be called on master so usecache will always be true.
I think the root problem here is that, HMaster extends HRegionServer, and they both uses TableDescriptors, but on HRegionServer it is read only and no cache. In general, we should make HMaster not extend HRegionServer in the future, then we could introduce a new ReadOnlyTableDescriptors to be used by HRegionServer and then a normal FSTableDescriptor for HMaster.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [hbase] Apache-HBase commented on pull request #2317: HBASE-24949 Optimize FSTableDescriptors.get to not always go to fs wh…
Posted by GitBox <gi...@apache.org>.
Apache-HBase commented on pull request #2317:
URL: https://github.com/apache/hbase/pull/2317#issuecomment-680787783
:broken_heart: **-1 overall**
| Vote | Subsystem | Runtime | Comment |
|:----:|----------:|--------:|:--------|
| +0 :ok: | reexec | 2m 1s | Docker mode activated. |
| -0 :warning: | yetus | 0m 3s | Unprocessed flag(s): --brief-report-file --spotbugs-strict-precheck --whitespace-eol-ignore-list --whitespace-tabs-ignore-list --quick-hadoopcheck |
||| _ Prechecks _ |
||| _ master Compile Tests _ |
| +1 :green_heart: | mvninstall | 4m 32s | master passed |
| +1 :green_heart: | compile | 1m 3s | master passed |
| +1 :green_heart: | shadedjars | 8m 0s | branch has no errors when building our shaded downstream artifacts. |
| +1 :green_heart: | javadoc | 0m 42s | master passed |
||| _ Patch Compile Tests _ |
| +1 :green_heart: | mvninstall | 4m 10s | the patch passed |
| +1 :green_heart: | compile | 1m 7s | the patch passed |
| +1 :green_heart: | javac | 1m 7s | the patch passed |
| +1 :green_heart: | shadedjars | 8m 12s | patch has no errors when building our shaded downstream artifacts. |
| +1 :green_heart: | javadoc | 0m 43s | the patch passed |
||| _ Other Tests _ |
| -1 :x: | unit | 7m 25s | hbase-server in the patch failed. |
| | | 39m 27s | |
| Subsystem | Report/Notes |
|----------:|:-------------|
| Docker | Client=19.03.12 Server=19.03.12 base: https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2317/1/artifact/yetus-jdk8-hadoop3-check/output/Dockerfile |
| GITHUB PR | https://github.com/apache/hbase/pull/2317 |
| Optional Tests | javac javadoc unit shadedjars compile |
| uname | Linux 681e9d2330c7 4.15.0-65-generic #74-Ubuntu SMP Tue Sep 17 17:06:04 UTC 2019 x86_64 x86_64 x86_64 GNU/Linux |
| Build tool | maven |
| Personality | dev-support/hbase-personality.sh |
| git revision | master / 01cf60067c |
| Default Java | 1.8.0_232 |
| unit | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2317/1/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-2317/1/testReport/ |
| Max. process+thread count | 596 (vs. ulimit of 12500) |
| modules | C: hbase-server U: hbase-server |
| Console output | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2317/1/console |
| versions | git=2.17.1 maven=(cecedd343002696d0abb50b32b541b8a6ba2883f) |
| Powered by | Apache Yetus 0.11.1 https://yetus.apache.org |
This message was automatically generated.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [hbase] Apache9 commented on a change in pull request #2317: HBASE-24949 Optimize FSTableDescriptors.get to not always go to fs wh…
Posted by GitBox <gi...@apache.org>.
Apache9 commented on a change in pull request #2317:
URL: https://github.com/apache/hbase/pull/2317#discussion_r478162122
##########
File path: hbase-server/src/main/java/org/apache/hadoop/hbase/util/FSTableDescriptors.java
##########
@@ -290,35 +272,43 @@ public TableDescriptor get(final TableName tablename)
* @see #get(org.apache.hadoop.hbase.TableName)
*/
@Override
- public Map<String, TableDescriptor> getByNamespace(String name)
- throws IOException {
+ public Map<String, TableDescriptor> getByNamespace(String name) throws IOException {
Map<String, TableDescriptor> htds = new TreeMap<>();
List<Path> tableDirs =
- FSUtils.getLocalTableDirs(fs, CommonFSUtils.getNamespaceDir(rootdir, name));
- for (Path d: tableDirs) {
- TableDescriptor htd = null;
- try {
- htd = get(CommonFSUtils.getTableName(d));
- } catch (FileNotFoundException fnfe) {
- // inability of retrieving one HTD shouldn't stop getting the remaining
- LOG.warn("Trouble retrieving htd", fnfe);
+ FSUtils.getLocalTableDirs(fs, CommonFSUtils.getNamespaceDir(rootdir, name));
+ for (Path d : tableDirs) {
+ TableDescriptor htd = get(CommonFSUtils.getTableName(d));
+ if (htd == null) {
+ continue;
}
- if (htd == null) continue;
htds.put(CommonFSUtils.getTableName(d).getNameAsString(), htd);
}
return htds;
}
- /**
- * Adds (or updates) the table descriptor to the FileSystem
- * and updates the local cache with it.
- */
@Override
- public void update(TableDescriptor htd) throws IOException {
+ public void update(TableDescriptor td, boolean cacheOnly) throws IOException {
Review comment:
In fact this will be called on master so usecache will always be true.
I think the root problem here is that, HMaster extends HRegionServer, and they both uses TableDescriptors, but on HRegionServer it is read only and no cache. In general, we should make HMaster not extend HRegionServer in the future, then we could introduce a new ReadOnlyTableDescriptors to be used by HRegionServer and then a normal FSTableDescriptor for HMaster.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [hbase] infraio commented on a change in pull request #2317: HBASE-24949 Optimize FSTableDescriptors.get to not always go to fs wh…
Posted by GitBox <gi...@apache.org>.
infraio commented on a change in pull request #2317:
URL: https://github.com/apache/hbase/pull/2317#discussion_r478134153
##########
File path: hbase-server/src/main/java/org/apache/hadoop/hbase/util/FSTableDescriptors.java
##########
@@ -290,35 +272,43 @@ public TableDescriptor get(final TableName tablename)
* @see #get(org.apache.hadoop.hbase.TableName)
*/
@Override
- public Map<String, TableDescriptor> getByNamespace(String name)
- throws IOException {
+ public Map<String, TableDescriptor> getByNamespace(String name) throws IOException {
Map<String, TableDescriptor> htds = new TreeMap<>();
List<Path> tableDirs =
- FSUtils.getLocalTableDirs(fs, CommonFSUtils.getNamespaceDir(rootdir, name));
- for (Path d: tableDirs) {
- TableDescriptor htd = null;
- try {
- htd = get(CommonFSUtils.getTableName(d));
- } catch (FileNotFoundException fnfe) {
- // inability of retrieving one HTD shouldn't stop getting the remaining
- LOG.warn("Trouble retrieving htd", fnfe);
+ FSUtils.getLocalTableDirs(fs, CommonFSUtils.getNamespaceDir(rootdir, name));
+ for (Path d : tableDirs) {
+ TableDescriptor htd = get(CommonFSUtils.getTableName(d));
+ if (htd == null) {
+ continue;
}
- if (htd == null) continue;
htds.put(CommonFSUtils.getTableName(d).getNameAsString(), htd);
}
return htds;
}
- /**
- * Adds (or updates) the table descriptor to the FileSystem
- * and updates the local cache with it.
- */
@Override
- public void update(TableDescriptor htd) throws IOException {
+ public void update(TableDescriptor td, boolean cacheOnly) throws IOException {
Review comment:
This will do nothing when cacheonly is true and usecache is false? It is weird...... How about throw UnsupportedOperationException for this case too?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [hbase] Apache-HBase commented on pull request #2317: HBASE-24949 Optimize FSTableDescriptors.get to not always go to fs wh…
Posted by GitBox <gi...@apache.org>.
Apache-HBase commented on pull request #2317:
URL: https://github.com/apache/hbase/pull/2317#issuecomment-681303574
:confetti_ball: **+1 overall**
| Vote | Subsystem | Runtime | Comment |
|:----:|----------:|--------:|:--------|
| +0 :ok: | reexec | 0m 30s | Docker mode activated. |
||| _ Prechecks _ |
| +1 :green_heart: | dupname | 0m 1s | 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. |
||| _ master Compile Tests _ |
| +1 :green_heart: | mvninstall | 3m 48s | master passed |
| +1 :green_heart: | checkstyle | 1m 9s | master passed |
| +1 :green_heart: | spotbugs | 2m 3s | master passed |
||| _ Patch Compile Tests _ |
| +1 :green_heart: | mvninstall | 3m 28s | the patch passed |
| -0 :warning: | checkstyle | 1m 6s | hbase-server: The patch generated 1 new + 168 unchanged - 11 fixed = 169 total (was 179) |
| +1 :green_heart: | whitespace | 0m 0s | The patch has no whitespace issues. |
| +1 :green_heart: | hadoopcheck | 11m 20s | Patch does not cause any errors with Hadoop 3.1.2 3.2.1. |
| +1 :green_heart: | spotbugs | 2m 10s | the patch passed |
||| _ Other Tests _ |
| +1 :green_heart: | asflicense | 0m 15s | The patch does not generate ASF License warnings. |
| | | 33m 6s | |
| Subsystem | Report/Notes |
|----------:|:-------------|
| Docker | Client=19.03.12 Server=19.03.12 base: https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2317/3/artifact/yetus-general-check/output/Dockerfile |
| GITHUB PR | https://github.com/apache/hbase/pull/2317 |
| Optional Tests | dupname asflicense spotbugs hadoopcheck hbaseanti checkstyle |
| uname | Linux ec9ec690933e 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 | master / f9ce1df3f2 |
| checkstyle | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2317/3/artifact/yetus-general-check/output/diff-checkstyle-hbase-server.txt |
| Max. process+thread count | 94 (vs. ulimit of 12500) |
| modules | C: hbase-server U: hbase-server |
| Console output | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2317/3/console |
| versions | git=2.17.1 maven=(cecedd343002696d0abb50b32b541b8a6ba2883f) spotbugs=3.1.12 |
| Powered by | Apache Yetus 0.11.1 https://yetus.apache.org |
This message was automatically generated.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [hbase] Apache9 commented on a change in pull request #2317: HBASE-24949 Optimize FSTableDescriptors.get to not always go to fs wh…
Posted by GitBox <gi...@apache.org>.
Apache9 commented on a change in pull request #2317:
URL: https://github.com/apache/hbase/pull/2317#discussion_r478455231
##########
File path: hbase-server/src/main/java/org/apache/hadoop/hbase/util/FSTableDescriptors.java
##########
@@ -290,35 +272,48 @@ public TableDescriptor get(final TableName tablename)
* @see #get(org.apache.hadoop.hbase.TableName)
*/
@Override
- public Map<String, TableDescriptor> getByNamespace(String name)
- throws IOException {
+ public Map<String, TableDescriptor> getByNamespace(String name) throws IOException {
Map<String, TableDescriptor> htds = new TreeMap<>();
List<Path> tableDirs =
- FSUtils.getLocalTableDirs(fs, CommonFSUtils.getNamespaceDir(rootdir, name));
- for (Path d: tableDirs) {
- TableDescriptor htd = null;
- try {
- htd = get(CommonFSUtils.getTableName(d));
- } catch (FileNotFoundException fnfe) {
- // inability of retrieving one HTD shouldn't stop getting the remaining
- LOG.warn("Trouble retrieving htd", fnfe);
+ FSUtils.getLocalTableDirs(fs, CommonFSUtils.getNamespaceDir(rootdir, name));
+ for (Path d : tableDirs) {
+ TableDescriptor htd = get(CommonFSUtils.getTableName(d));
+ if (htd == null) {
+ continue;
}
- if (htd == null) continue;
htds.put(CommonFSUtils.getTableName(d).getNameAsString(), htd);
}
return htds;
}
- /**
- * Adds (or updates) the table descriptor to the FileSystem
- * and updates the local cache with it.
- */
@Override
- public void update(TableDescriptor htd) throws IOException {
+ public void update(TableDescriptor td, boolean cacheOnly) throws IOException {
+ // TODO: in fact this method will only be called at master side, so fsreadonly and usecache will
+ // always be true. In general, we'd better have a ReadOnlyFSTableDesciptors for HRegionServer
+ // but now, HMaster extends HRegionServer, so unless making use of generic, we can not have
+ // different implementations for HMaster and HRegionServer. Revisit this when we make HMaster
+ // not extend HRegionServer in the future.
if (fsreadonly) {
- throw new NotImplementedException("Cannot add a table descriptor - in read only mode");
+ throw new UnsupportedOperationException("Cannot add a table descriptor - in read only mode");
+ }
+ if (!cacheOnly) {
+ updateTableDesciptor(td);
+ }
+ if (usecache) {
+ this.cache.put(td.getTableName(), td);
}
- updateTableDescriptor(htd);
+ }
+
+ @VisibleForTesting
+ Path updateTableDesciptor(TableDescriptor td) throws IOException {
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [hbase] Apache-HBase commented on pull request #2317: HBASE-24949 Optimize FSTableDescriptors.get to not always go to fs wh…
Posted by GitBox <gi...@apache.org>.
Apache-HBase commented on pull request #2317:
URL: https://github.com/apache/hbase/pull/2317#issuecomment-680789230
:broken_heart: **-1 overall**
| Vote | Subsystem | Runtime | Comment |
|:----:|----------:|--------:|:--------|
| +0 :ok: | reexec | 0m 43s | Docker mode activated. |
| -0 :warning: | yetus | 0m 2s | Unprocessed flag(s): --brief-report-file --spotbugs-strict-precheck --whitespace-eol-ignore-list --whitespace-tabs-ignore-list --quick-hadoopcheck |
||| _ Prechecks _ |
||| _ master Compile Tests _ |
| +1 :green_heart: | mvninstall | 5m 2s | master passed |
| +1 :green_heart: | compile | 1m 15s | master passed |
| +1 :green_heart: | shadedjars | 7m 35s | branch has no errors when building our shaded downstream artifacts. |
| -0 :warning: | javadoc | 0m 51s | hbase-server in master failed. |
||| _ Patch Compile Tests _ |
| +1 :green_heart: | mvninstall | 4m 41s | the patch passed |
| +1 :green_heart: | compile | 1m 19s | the patch passed |
| +1 :green_heart: | javac | 1m 19s | the patch passed |
| +1 :green_heart: | shadedjars | 8m 3s | patch has no errors when building our shaded downstream artifacts. |
| -0 :warning: | javadoc | 0m 43s | hbase-server in the patch failed. |
||| _ Other Tests _ |
| -1 :x: | unit | 8m 18s | hbase-server in the patch failed. |
| | | 39m 45s | |
| Subsystem | Report/Notes |
|----------:|:-------------|
| Docker | Client=19.03.12 Server=19.03.12 base: https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2317/1/artifact/yetus-jdk11-hadoop3-check/output/Dockerfile |
| GITHUB PR | https://github.com/apache/hbase/pull/2317 |
| Optional Tests | javac javadoc unit shadedjars compile |
| uname | Linux 24a7a8af610a 4.15.0-60-generic #67-Ubuntu SMP Thu Aug 22 16:55:30 UTC 2019 x86_64 x86_64 x86_64 GNU/Linux |
| Build tool | maven |
| Personality | dev-support/hbase-personality.sh |
| git revision | master / 01cf60067c |
| Default Java | 2020-01-14 |
| javadoc | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2317/1/artifact/yetus-jdk11-hadoop3-check/output/branch-javadoc-hbase-server.txt |
| javadoc | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2317/1/artifact/yetus-jdk11-hadoop3-check/output/patch-javadoc-hbase-server.txt |
| unit | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2317/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-2317/1/testReport/ |
| Max. process+thread count | 741 (vs. ulimit of 12500) |
| modules | C: hbase-server U: hbase-server |
| Console output | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2317/1/console |
| versions | git=2.17.1 maven=(cecedd343002696d0abb50b32b541b8a6ba2883f) |
| Powered by | Apache Yetus 0.11.1 https://yetus.apache.org |
This message was automatically generated.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [hbase] infraio commented on a change in pull request #2317: HBASE-24949 Optimize FSTableDescriptors.get to not always go to fs wh…
Posted by GitBox <gi...@apache.org>.
infraio commented on a change in pull request #2317:
URL: https://github.com/apache/hbase/pull/2317#discussion_r478246348
##########
File path: hbase-server/src/main/java/org/apache/hadoop/hbase/util/FSTableDescriptors.java
##########
@@ -290,35 +272,43 @@ public TableDescriptor get(final TableName tablename)
* @see #get(org.apache.hadoop.hbase.TableName)
*/
@Override
- public Map<String, TableDescriptor> getByNamespace(String name)
- throws IOException {
+ public Map<String, TableDescriptor> getByNamespace(String name) throws IOException {
Map<String, TableDescriptor> htds = new TreeMap<>();
List<Path> tableDirs =
- FSUtils.getLocalTableDirs(fs, CommonFSUtils.getNamespaceDir(rootdir, name));
- for (Path d: tableDirs) {
- TableDescriptor htd = null;
- try {
- htd = get(CommonFSUtils.getTableName(d));
- } catch (FileNotFoundException fnfe) {
- // inability of retrieving one HTD shouldn't stop getting the remaining
- LOG.warn("Trouble retrieving htd", fnfe);
+ FSUtils.getLocalTableDirs(fs, CommonFSUtils.getNamespaceDir(rootdir, name));
+ for (Path d : tableDirs) {
+ TableDescriptor htd = get(CommonFSUtils.getTableName(d));
+ if (htd == null) {
+ continue;
}
- if (htd == null) continue;
htds.put(CommonFSUtils.getTableName(d).getNameAsString(), htd);
}
return htds;
}
- /**
- * Adds (or updates) the table descriptor to the FileSystem
- * and updates the local cache with it.
- */
@Override
- public void update(TableDescriptor htd) throws IOException {
+ public void update(TableDescriptor td, boolean cacheOnly) throws IOException {
Review comment:
So left a TODO here? Maybe make this more clear in the future.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [hbase] Apache9 commented on pull request #2317: HBASE-24949 Optimize FSTableDescriptors.get to not always go to fs wh…
Posted by GitBox <gi...@apache.org>.
Apache9 commented on pull request #2317:
URL: https://github.com/apache/hbase/pull/2317#issuecomment-683297792
OK we run out of space again... Anyway, let me merge since the UTs are all fine now.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org