You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@tajo.apache.org by hyunsik <gi...@git.apache.org> on 2015/12/14 07:14:05 UTC
[GitHub] tajo pull request: TAJO-1940: Implement HBaseTablespace::getTableV...
GitHub user hyunsik opened a pull request:
https://github.com/apache/tajo/pull/910
TAJO-1940: Implement HBaseTablespace::getTableVolume() method.
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/hyunsik/tajo TAJO-1940
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/tajo/pull/910.patch
To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:
This closes #910
----
commit 367dcbc3162c42f2eddec8d544ac246150760220
Author: Hyunsik Choi <hy...@apache.org>
Date: 2015-11-26T05:41:46Z
TAJO-1990: Refine some parts in HBaseTablespace.
commit d967eeb38039fce1f100adb361c58ef9dd7743da
Author: Hyunsik Choi <hy...@apache.org>
Date: 2015-12-03T06:15:44Z
TAJO-2005: Add TableStatUpdateRewriter.
commit ebaf48eecf505489de611153b9a364227edf8307
Author: Hyunsik Choi <hy...@apache.org>
Date: 2015-12-03T06:49:49Z
Merge branch 'master' of https://git-wip-us.apache.org/repos/asf/tajo into TAJO-1990
commit db6a590890b03527658bd6b5876b2234781eb6fe
Author: Hyunsik Choi <hy...@apache.org>
Date: 2015-12-03T07:49:52Z
Merge branch 'master' of https://git-wip-us.apache.org/repos/asf/tajo into TAJO-2005
commit a16aeff172eb8e61bb57ac120619d81f42069a0d
Author: Hyunsik Choi <hy...@apache.org>
Date: 2015-12-03T07:50:05Z
TAJO-2007: By default, Optimizer should use the table volume in TableStat.
commit e70a04d4561314232f68c8361e34f61e9f260a25
Author: Hyunsik Choi <hy...@apache.org>
Date: 2015-12-03T07:57:07Z
Merge branch 'master' of https://git-wip-us.apache.org/repos/asf/tajo into TAJO-2007
commit 60b714bde6a7ddbcb238e8e39095f55f283eb589
Author: Hyunsik Choi <hy...@apache.org>
Date: 2015-12-03T08:00:09Z
Removed unused parameters.
commit 82e71774095d634bebec54187051f139aa06d456
Author: Hyunsik Choi <hy...@apache.org>
Date: 2015-12-03T08:17:40Z
TAJO-1991: Tablespace::getVolume should take filter predication.
commit 66c5278bf1d98ff691802a9cbe6eedf9a7dd08d7
Author: Hyunsik Choi <hy...@apache.org>
Date: 2015-12-03T08:20:46Z
Add guava to pom.
commit edd604c823d5a5d183b8860d25a05c05a86ce3ae
Author: Hyunsik Choi <hy...@apache.org>
Date: 2015-12-03T08:37:44Z
Fixed errors.
commit a2ab0e634ed59665c4a4e27bd6cef4347549865a
Author: Hyunsik Choi <hy...@apache.org>
Date: 2015-12-03T08:47:38Z
Changed Optional to JDK8's one.
commit c342c4b9b5d7365d7df5d9b4a977082f18430a62
Author: Hyunsik Choi <hy...@apache.org>
Date: 2015-12-03T09:00:56Z
Fixed undeterministic results.
commit ee304104cde1edad1553d82e2d9f1dbd06e06e9b
Author: Hyunsik Choi <hy...@apache.org>
Date: 2015-12-04T08:11:44Z
Set the default volume.
commit c3d928f67bca2a386c46039c9e6c482f59a2243c
Author: Hyunsik Choi <hy...@apache.org>
Date: 2015-12-04T08:12:17Z
Merge branch 'master' of https://git-wip-us.apache.org/repos/asf/tajo into TAJO-1991
commit fce17c3294c222f24deb1b94b93b75bb5ae8dbc7
Author: Hyunsik Choi <hy...@apache.org>
Date: 2015-12-04T08:39:44Z
Merge branch 'master' of https://git-wip-us.apache.org/repos/asf/tajo into TAJO-1991
commit b25688fd1d16fc37b5710ce6c96c28f9b0fe46d9
Author: Hyunsik Choi <hy...@apache.org>
Date: 2015-12-04T08:57:31Z
Removed invalid imports.
commit 1a931c9e415ab3f355a32d1ffdd88f9e660398cb
Author: Hyunsik Choi <hy...@apache.org>
Date: 2015-12-04T18:26:42Z
Updated the test result.
commit 1432e6c726a3f10586b241e6704fb4c303ddfa32
Author: Hyunsik Choi <hy...@apache.org>
Date: 2015-12-13T20:49:51Z
Merge branch 'master' of https://git-wip-us.apache.org/repos/asf/tajo into TAJO-1991
commit ead71440e8e06b30a1a3e13539fc1f5df221f6ee
Author: Hyunsik Choi <hy...@apache.org>
Date: 2015-12-12T21:35:48Z
Fixed unit tests.
commit 5ffb0826ce4bc2da18b7d0fe18588f0b1bc2b932
Author: Hyunsik Choi <hy...@apache.org>
Date: 2015-12-13T02:22:58Z
Fixed test failures.
commit 8bcb0e7bae10fa4e73ab9aa7a9739ec881047d6c
Author: Hyunsik Choi <hy...@apache.org>
Date: 2015-12-13T20:35:18Z
Recover the change log and fix config key.
commit b0749827fb497a2aa290705e49d636cac5f48e03
Author: Hyunsik Choi <hy...@apache.org>
Date: 2015-12-13T23:18:25Z
Merge branch 'master' of https://git-wip-us.apache.org/repos/asf/tajo into TAJO-1990
commit 27ec5c5f6c173a1e8d2e13f9c0a9534fa6dd7ad9
Author: Hyunsik Choi <hy...@apache.org>
Date: 2015-12-13T23:46:49Z
Fixed the test failure due to import HConstant.
commit 4a007983a9cb2273571c4bf14e65e74935f9acc8
Author: Hyunsik Choi <hy...@apache.org>
Date: 2015-12-14T00:17:27Z
Merge branch 'TAJO-1990' of github.com:hyunsik/tajo into TAJO-1940
commit 7eefce5e6bd90758d9697c5a3096254f04da9489
Author: Hyunsik Choi <hy...@apache.org>
Date: 2015-12-14T01:21:00Z
Refactor HBaseTablespace::getSplits() and implemented getTableVolume().
----
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] tajo pull request: TAJO-1940: Implement HBaseTablespace::getTableV...
Posted by jihoonson <gi...@git.apache.org>.
Github user jihoonson commented on a diff in the pull request:
https://github.com/apache/tajo/pull/910#discussion_r50493788
--- Diff: tajo-core-tests/src/test/java/org/apache/tajo/engine/query/TestHBaseTable.java ---
@@ -210,7 +210,62 @@ public void testCreateExternalHBaseTable() throws Exception {
} finally {
TablespaceManager.addTableSpaceForTest(existing.get());
}
+ }
+ private void putData(HTableInterface htable, int rownum) throws IOException {
+ for (int i = 0; i < rownum; i++) {
+ Put put = new Put(String.valueOf(i).getBytes());
+ put.add("col1".getBytes(), "a".getBytes(), ("a-" + i).getBytes());
+ put.add("col1".getBytes(), "b".getBytes(), ("b-" + i).getBytes());
+ put.add("col2".getBytes(), "k1".getBytes(), ("k1-" + i).getBytes());
+ put.add("col2".getBytes(), "k2".getBytes(), ("k2-" + i).getBytes());
+ put.add("col3".getBytes(), "b".getBytes(), ("b-" + i).getBytes());
+ htable.put(put);
+ }
+ }
+
+ @Test
+ public void testGetTableVolume() throws Exception {
--- End diff --
This test looks good, but I'm concerned with its location.
IMO, more proper class which should contain this test is ```TestHBaseTablespace```, but, as you already know, that class doesn't involve running a test hbase cluster.
So, if we move this test to ```TestHBaseTablespace```, we also need to launch another hbase cluster which can increase our test time.
What do you think?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] tajo pull request: TAJO-1940: Implement HBaseTablespace::getTableV...
Posted by hyunsik <gi...@git.apache.org>.
Github user hyunsik commented on a diff in the pull request:
https://github.com/apache/tajo/pull/910#discussion_r50501497
--- Diff: tajo-storage/tajo-storage-hbase/src/main/java/org/apache/tajo/storage/hbase/RegionSizeCalculator.java ---
@@ -0,0 +1,147 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.tajo.storage.hbase;
+
+import java.io.IOException;
+import java.util.Arrays;
+import java.util.Collection;
+import java.util.Collections;
+import java.util.List;
+import java.util.Map;
+import java.util.Set;
+import java.util.TreeMap;
+import java.util.TreeSet;
+
+import org.apache.commons.logging.Log;
+import org.apache.commons.logging.LogFactory;
+import org.apache.hadoop.conf.Configuration;
+import org.apache.hadoop.hbase.ClusterStatus;
+import org.apache.hadoop.hbase.HRegionLocation;
+import org.apache.hadoop.hbase.RegionLoad;
+import org.apache.hadoop.hbase.ServerLoad;
+import org.apache.hadoop.hbase.ServerName;
+import org.apache.hadoop.hbase.classification.InterfaceAudience;
+import org.apache.hadoop.hbase.classification.InterfaceStability;
+import org.apache.hadoop.hbase.client.Admin;
+import org.apache.hadoop.hbase.client.HBaseAdmin;
+import org.apache.hadoop.hbase.client.HTable;
+import org.apache.hadoop.hbase.client.RegionLocator;
+import org.apache.hadoop.hbase.util.Bytes;
+
+/**
+ * Computes size of each region for given table and given column families.
+ * The value is used by MapReduce for better scheduling.
+ * */
+@InterfaceStability.Evolving
+@InterfaceAudience.Private
+public class RegionSizeCalculator {
+
+ private static final Log LOG = LogFactory.getLog(RegionSizeCalculator.class);
+
+ /**
+ * Maps each region to its size in bytes.
+ * */
+ private final Map<byte[], Long> sizeMap = new TreeMap<byte[], Long>(Bytes.BYTES_COMPARATOR);
+
+ static final String ENABLE_REGIONSIZECALCULATOR = "hbase.regionsizecalculator.enable";
+
+ /**
+ * Computes size of each region for table and given column families.
+ *
+ * @deprecated Use {@link #RegionSizeCalculator(RegionLocator, Admin)} instead.
+ */
+ @Deprecated
+ public RegionSizeCalculator(HTable table) throws IOException {
+ HBaseAdmin admin = new HBaseAdmin(table.getConfiguration());
+ try {
+ init(table.getRegionLocator(), admin);
+ } finally {
+ admin.close();
+ }
+ }
+
+ /**
+ * Computes size of each region for table and given column families.
+ * */
+ public RegionSizeCalculator(RegionLocator regionLocator, Admin admin) throws IOException {
+ init(regionLocator, admin);
+ }
+
+ private void init(RegionLocator regionLocator, Admin admin)
+ throws IOException {
+ if (!enabled(admin.getConfiguration())) {
+ LOG.info("Region size calculation disabled.");
+ return;
+ }
+
+ LOG.info("Calculating region sizes for table \"" + regionLocator.getName() + "\".");
+
+ //get regions for table
+ List<HRegionLocation> tableRegionInfos = regionLocator.getAllRegionLocations();
+ Set<byte[]> tableRegions = new TreeSet<byte[]>(Bytes.BYTES_COMPARATOR);
+ for (HRegionLocation regionInfo : tableRegionInfos) {
+ tableRegions.add(regionInfo.getRegionInfo().getRegionName());
+ }
+
+ ClusterStatus clusterStatus = admin.getClusterStatus();
+ Collection<ServerName> servers = clusterStatus.getServers();
+ final long megaByte = 1024L * 1024L;
+
+ //iterate all cluster regions, filter regions from our table and compute their size
+ for (ServerName serverName: servers) {
+ ServerLoad serverLoad = clusterStatus.getLoad(serverName);
+
+ for (RegionLoad regionLoad: serverLoad.getRegionsLoad().values()) {
+ byte[] regionId = regionLoad.getName();
+
+ if (tableRegions.contains(regionId)) {
+
+ long regionSizeBytes = (regionLoad.getStorefileSizeMB() + regionLoad.getMemStoreSizeMB()) * megaByte;
--- End diff --
The original RegionSizeCalculator does not concern the size of memory store. So, I forked it and modified the class to use region files as well as memory store.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] tajo pull request: TAJO-1940: Implement HBaseTablespace::getTableV...
Posted by hyunsik <gi...@git.apache.org>.
Github user hyunsik commented on the pull request:
https://github.com/apache/tajo/pull/910#issuecomment-173433779
Added unit tests and rebased the patch.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] tajo pull request: TAJO-1940: Implement HBaseTablespace::getTableV...
Posted by hyunsik <gi...@git.apache.org>.
Github user hyunsik commented on the pull request:
https://github.com/apache/tajo/pull/910#issuecomment-175888696
Thank you. I've added some note for that.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] tajo pull request: TAJO-1940: Implement HBaseTablespace::getTableV...
Posted by jihoonson <gi...@git.apache.org>.
Github user jihoonson commented on the pull request:
https://github.com/apache/tajo/pull/910#issuecomment-175867700
I've tested the latest patch manually.
Before you commit, please add some comments about the difference between the original RegionSizeCalculator and the copied one on https://github.com/apache/tajo/pull/910#discussion-diff-50492684.
Here is my +1.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] tajo pull request: TAJO-1940: Implement HBaseTablespace::getTableV...
Posted by hyunsik <gi...@git.apache.org>.
Github user hyunsik commented on the pull request:
https://github.com/apache/tajo/pull/910#issuecomment-173803616
Yes, that's why I placed the test in TestHbaseTable. As you know, Hbase cluster requires lots of additional memory, and we are suffering from memory problem of unit tests.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] tajo pull request: TAJO-1940: Implement HBaseTablespace::getTableV...
Posted by hyunsik <gi...@git.apache.org>.
Github user hyunsik commented on the pull request:
https://github.com/apache/tajo/pull/910#issuecomment-175812809
Rebased and reflected a comment.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] tajo pull request: TAJO-1940: Implement HBaseTablespace::getTableV...
Posted by hyunsik <gi...@git.apache.org>.
Github user hyunsik commented on a diff in the pull request:
https://github.com/apache/tajo/pull/910#discussion_r50501505
--- Diff: tajo-storage/tajo-storage-hbase/src/main/java/org/apache/tajo/storage/hbase/HBaseTablespace.java ---
@@ -101,8 +104,19 @@ public Configuration getHbaseConf() {
}
@Override
- public long getTableVolume(URI uri, Optional<EvalNode> filter) throws UnsupportedException {
- throw new UnsupportedException();
+ public long getTableVolume(TableDesc table, Optional<EvalNode> filter) {
+ long totalVolume;
+ try {
+ totalVolume = getRawSplits("", table, filter.orElse(null)).stream()
+ .map(f -> f.getLength())
+ .filter(size -> size > 0) // eliminate unknown sizes (-1)
+ .reduce(0L, (sum, size) -> sum + size);
--- End diff --
Thanks. I'll fix it.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] tajo pull request: TAJO-1940: Implement HBaseTablespace::getTableV...
Posted by jihoonson <gi...@git.apache.org>.
Github user jihoonson commented on a diff in the pull request:
https://github.com/apache/tajo/pull/910#discussion_r50492684
--- Diff: tajo-storage/tajo-storage-hbase/src/main/java/org/apache/tajo/storage/hbase/RegionSizeCalculator.java ---
@@ -0,0 +1,147 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.tajo.storage.hbase;
+
+import java.io.IOException;
+import java.util.Arrays;
+import java.util.Collection;
+import java.util.Collections;
+import java.util.List;
+import java.util.Map;
+import java.util.Set;
+import java.util.TreeMap;
+import java.util.TreeSet;
+
+import org.apache.commons.logging.Log;
+import org.apache.commons.logging.LogFactory;
+import org.apache.hadoop.conf.Configuration;
+import org.apache.hadoop.hbase.ClusterStatus;
+import org.apache.hadoop.hbase.HRegionLocation;
+import org.apache.hadoop.hbase.RegionLoad;
+import org.apache.hadoop.hbase.ServerLoad;
+import org.apache.hadoop.hbase.ServerName;
+import org.apache.hadoop.hbase.classification.InterfaceAudience;
+import org.apache.hadoop.hbase.classification.InterfaceStability;
+import org.apache.hadoop.hbase.client.Admin;
+import org.apache.hadoop.hbase.client.HBaseAdmin;
+import org.apache.hadoop.hbase.client.HTable;
+import org.apache.hadoop.hbase.client.RegionLocator;
+import org.apache.hadoop.hbase.util.Bytes;
+
+/**
+ * Computes size of each region for given table and given column families.
+ * The value is used by MapReduce for better scheduling.
+ * */
+@InterfaceStability.Evolving
+@InterfaceAudience.Private
+public class RegionSizeCalculator {
+
+ private static final Log LOG = LogFactory.getLog(RegionSizeCalculator.class);
+
+ /**
+ * Maps each region to its size in bytes.
+ * */
+ private final Map<byte[], Long> sizeMap = new TreeMap<byte[], Long>(Bytes.BYTES_COMPARATOR);
+
+ static final String ENABLE_REGIONSIZECALCULATOR = "hbase.regionsizecalculator.enable";
+
+ /**
+ * Computes size of each region for table and given column families.
+ *
+ * @deprecated Use {@link #RegionSizeCalculator(RegionLocator, Admin)} instead.
+ */
+ @Deprecated
+ public RegionSizeCalculator(HTable table) throws IOException {
+ HBaseAdmin admin = new HBaseAdmin(table.getConfiguration());
+ try {
+ init(table.getRegionLocator(), admin);
+ } finally {
+ admin.close();
+ }
+ }
+
+ /**
+ * Computes size of each region for table and given column families.
+ * */
+ public RegionSizeCalculator(RegionLocator regionLocator, Admin admin) throws IOException {
+ init(regionLocator, admin);
+ }
+
+ private void init(RegionLocator regionLocator, Admin admin)
+ throws IOException {
+ if (!enabled(admin.getConfiguration())) {
+ LOG.info("Region size calculation disabled.");
+ return;
+ }
+
+ LOG.info("Calculating region sizes for table \"" + regionLocator.getName() + "\".");
+
+ //get regions for table
+ List<HRegionLocation> tableRegionInfos = regionLocator.getAllRegionLocations();
+ Set<byte[]> tableRegions = new TreeSet<byte[]>(Bytes.BYTES_COMPARATOR);
+ for (HRegionLocation regionInfo : tableRegionInfos) {
+ tableRegions.add(regionInfo.getRegionInfo().getRegionName());
+ }
+
+ ClusterStatus clusterStatus = admin.getClusterStatus();
+ Collection<ServerName> servers = clusterStatus.getServers();
+ final long megaByte = 1024L * 1024L;
+
+ //iterate all cluster regions, filter regions from our table and compute their size
+ for (ServerName serverName: servers) {
+ ServerLoad serverLoad = clusterStatus.getLoad(serverName);
+
+ for (RegionLoad regionLoad: serverLoad.getRegionsLoad().values()) {
+ byte[] regionId = regionLoad.getName();
+
+ if (tableRegions.contains(regionId)) {
+
+ long regionSizeBytes = (regionLoad.getStorefileSizeMB() + regionLoad.getMemStoreSizeMB()) * megaByte;
--- End diff --
This line looks to be improved to consider the mem store size for region size calculation. Would you leave some comments?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] tajo pull request: TAJO-1940: Implement HBaseTablespace::getTableV...
Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:
https://github.com/apache/tajo/pull/910
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] tajo pull request: TAJO-1940: Implement HBaseTablespace::getTableV...
Posted by jihoonson <gi...@git.apache.org>.
Github user jihoonson commented on a diff in the pull request:
https://github.com/apache/tajo/pull/910#discussion_r49050091
--- Diff: tajo-storage/tajo-storage-hbase/src/main/java/org/apache/tajo/storage/hbase/HBaseTablespace.java ---
@@ -100,8 +102,19 @@ public Configuration getHbaseConf() {
}
@Override
- public long getTableVolume(URI uri, Optional<EvalNode> filter) throws UnsupportedException {
- throw new UnsupportedException();
+ public long getTableVolume(TableDesc table, Optional<EvalNode> filter) throws UnsupportedException {
--- End diff --
Please remove ```UnsupportedException```.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] tajo pull request: TAJO-1940: Implement HBaseTablespace::getTableV...
Posted by hyunsik <gi...@git.apache.org>.
Github user hyunsik commented on the pull request:
https://github.com/apache/tajo/pull/910#issuecomment-169565627
rebased.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] tajo pull request: TAJO-1940: Implement HBaseTablespace::getTableV...
Posted by jihoonson <gi...@git.apache.org>.
Github user jihoonson commented on a diff in the pull request:
https://github.com/apache/tajo/pull/910#discussion_r50493447
--- Diff: tajo-storage/tajo-storage-hbase/src/main/java/org/apache/tajo/storage/hbase/HBaseTablespace.java ---
@@ -101,8 +104,19 @@ public Configuration getHbaseConf() {
}
@Override
- public long getTableVolume(URI uri, Optional<EvalNode> filter) throws UnsupportedException {
- throw new UnsupportedException();
+ public long getTableVolume(TableDesc table, Optional<EvalNode> filter) {
+ long totalVolume;
+ try {
+ totalVolume = getRawSplits("", table, filter.orElse(null)).stream()
+ .map(f -> f.getLength())
+ .filter(size -> size > 0) // eliminate unknown sizes (-1)
+ .reduce(0L, (sum, size) -> sum + size);
--- End diff --
This line can be more simplified into ```.reduce(0L, Long::sum)```.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] tajo pull request: TAJO-1940: Implement HBaseTablespace::getTableV...
Posted by jihoonson <gi...@git.apache.org>.
Github user jihoonson commented on the pull request:
https://github.com/apache/tajo/pull/910#issuecomment-169607168
Thanks @hyunsik. This patch looks good to me overall. I have one suggestion.
It looks that ```getTableVolume()``` works well, but we need to verify its behaviour. How about adding any test?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---