You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@accumulo.apache.org by GitBox <gi...@apache.org> on 2020/12/03 21:42:48 UTC
[GitHub] [accumulo] milleruntime opened a new pull request #1821: Create listtablets shell command. Closes #1317
milleruntime opened a new pull request #1821:
URL: https://github.com/apache/accumulo/pull/1821
* New command for debugging tablets called listtablets
* Added getLiveTServers() to TabletMetadata for generating a list of
tservers that currently have a lock in ZK, similar to master.
* The list of live tservers is passed to TabletMetadata in order to get
the current state of a tablet
* Command will print one line for every tablet in a table
* Created TabletMetadataIT for testing getLiveTServers()
Co-authored-by: EdColeman <de...@etcoleman.com>
----------------------------------------------------------------
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] [accumulo] milleruntime commented on pull request #1821: Create listtablets shell command. Closes #1317
Posted by GitBox <gi...@apache.org>.
milleruntime commented on pull request #1821:
URL: https://github.com/apache/accumulo/pull/1821#issuecomment-754193165
FYI here is an example of output from the command using some test data:
<pre>
root@uno> listtablets -t test_ingest -h
2021-01-04T15:12:47,663 [Shell.audit] INFO : root@uno> listtablets -t test_ingest -h
NUM TABLET_DIR FILES WALS ENTRIES SIZE STATUS LOCATION ID START (Exclusive) END
TABLE: test_ingest
1 t-0000007 1 0 60 552 HOSTED CURRENT:ip-10-113-12-25:9997 2 -INF row_0000000005
2 t-0000006 1 0 500 2.71K HOSTED CURRENT:ip-10-113-12-25:9997 2 row_0000000005 row_0000000055
3 t-0000008 1 0 5.00K 24.74K HOSTED CURRENT:ip-10-113-12-25:9997 2 row_0000000055 row_0000000555
4 default_tablet 1 0 4.44K 22.01K HOSTED CURRENT:ip-10-113-12-25:9997 2 row_0000000555 +INF
root@uno> listtablets -t accumulo.metadata
2021-01-04T15:13:21,750 [Shell.audit] INFO : root@uno> listtablets -t accumulo.metadata
NUM TABLET_DIR FILES WALS ENTRIES SIZE STATUS LOCATION ID START (Exclusive) END
TABLE: accumulo.metadata
1 table_info 2 0 7 524 HOSTED CURRENT:ip-10-113-12-25:9997 !0 -INF ~
2 default_tablet 0 0 0 0 HOSTED CURRENT:ip-10-113-12-25:9997 !0 ~ +INF
</pre>
----------------------------------------------------------------
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] [accumulo] ctubbsii commented on pull request #1821: Create listtablets shell command. Closes #1317
Posted by GitBox <gi...@apache.org>.
ctubbsii commented on pull request #1821:
URL: https://github.com/apache/accumulo/pull/1821#issuecomment-754192077
> @ctubbsii I think this PR good to go. Did you have any other feedback before I merge?
No. Go ahead.
----------------------------------------------------------------
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] [accumulo] milleruntime commented on pull request #1821: Create listtablets shell command. Closes #1317
Posted by GitBox <gi...@apache.org>.
milleruntime commented on pull request #1821:
URL: https://github.com/apache/accumulo/pull/1821#issuecomment-754181051
@ctubbsii I think this PR good to go. Did you have any other feedback before I merge?
----------------------------------------------------------------
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] [accumulo] milleruntime commented on a change in pull request #1821: Create listtablets shell command. Closes #1317
Posted by GitBox <gi...@apache.org>.
milleruntime commented on a change in pull request #1821:
URL: https://github.com/apache/accumulo/pull/1821#discussion_r542760915
##########
File path: core/src/main/java/org/apache/accumulo/core/metadata/schema/TabletMetadata.java
##########
@@ -435,4 +445,34 @@ static TabletMetadata create(String id, String prevEndRow, String endRow) {
te.fetchedCols = EnumSet.of(ColumnType.PREV_ROW);
return te;
}
+
+ public static synchronized Set<TServerInstance> getLiveTServers(ClientContext context) {
+ final Set<TServerInstance> currentServers = new HashSet<>();
+
+ final String path = context.getZooKeeperRoot() + Constants.ZTSERVERS;
+
+ HashSet<String> all = new HashSet<>(context.getZooCache().getChildren(path));
+
+ for (String zPath : all) {
+ checkServer(context, currentServers, path, zPath);
+ }
Review comment:
I agree that mutating a data structure passed as a param can be deceptive. For me personally, I typically find streams to be less readable. I was just using what was there to start with since some of the code used in the locking I don't fully understand. I do like your non-stream suggestion, I will use that.
----------------------------------------------------------------
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] [accumulo] ctubbsii commented on a change in pull request #1821: Create listtablets shell command. Closes #1317
Posted by GitBox <gi...@apache.org>.
ctubbsii commented on a change in pull request #1821:
URL: https://github.com/apache/accumulo/pull/1821#discussion_r537801024
##########
File path: test/src/main/java/org/apache/accumulo/test/functional/TabletMetadataIT.java
##########
@@ -0,0 +1,94 @@
+/*
+ * 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.accumulo.test.functional;
+
+import static org.apache.accumulo.fate.util.UtilWaitThread.sleepUninterruptibly;
+import static org.apache.accumulo.minicluster.ServerType.TABLET_SERVER;
+import static org.junit.Assert.assertEquals;
+
+import java.util.Set;
+import java.util.concurrent.TimeUnit;
+
+import org.apache.accumulo.core.client.Accumulo;
+import org.apache.accumulo.core.client.AccumuloClient;
+import org.apache.accumulo.core.clientImpl.ClientContext;
+import org.apache.accumulo.core.metadata.TServerInstance;
+import org.apache.accumulo.core.metadata.schema.TabletMetadata;
+import org.apache.accumulo.harness.MiniClusterConfigurationCallback;
+import org.apache.accumulo.harness.SharedMiniClusterBase;
+import org.apache.accumulo.miniclusterImpl.MiniAccumuloConfigImpl;
+import org.apache.hadoop.conf.Configuration;
+import org.junit.AfterClass;
+import org.junit.BeforeClass;
+import org.junit.Test;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+/**
+ * Tests features of the Ample TabletMetadata class that can't be tested in TabletMetadataTest
+ */
+public class TabletMetadataIT extends SharedMiniClusterBase {
+ private static final Logger log = LoggerFactory.getLogger(TabletMetadataIT.class);
+ private static final int NUM_TSERVERS = 4;
+
+ @BeforeClass
+ public static void setup() throws Exception {
+ SharedMiniClusterBase.startMiniClusterWithConfig(new Callback());
+ }
+
+ @AfterClass
+ public static void teardown() {
+ SharedMiniClusterBase.stopMiniCluster();
+ }
+
+ @Override
+ protected int defaultTimeoutSeconds() {
+ return 120;
+ }
+
+ private static class Callback implements MiniClusterConfigurationCallback {
+ @Override
+ public void configureMiniCluster(MiniAccumuloConfigImpl cfg, Configuration conf) {
+ cfg.setNumTservers(NUM_TSERVERS);
+ }
+ }
Review comment:
I feel like we have some sort of ConfigurableMacIT that is our normal way of doing this. Will that work here instead of this callback?
##########
File path: core/src/main/java/org/apache/accumulo/core/metadata/schema/TabletMetadata.java
##########
@@ -435,4 +445,34 @@ static TabletMetadata create(String id, String prevEndRow, String endRow) {
te.fetchedCols = EnumSet.of(ColumnType.PREV_ROW);
return te;
}
+
+ public static synchronized Set<TServerInstance> getLiveTServers(ClientContext context) {
+ final Set<TServerInstance> currentServers = new HashSet<>();
+
+ final String path = context.getZooKeeperRoot() + Constants.ZTSERVERS;
+
+ HashSet<String> all = new HashSet<>(context.getZooCache().getChildren(path));
+
+ for (String zPath : all) {
+ checkServer(context, currentServers, path, zPath);
+ }
Review comment:
This pattern of populating `currentServers` inside the other method, `checkServer`, after looping over `all` can be improved with something like:
```java
var children = context.getZooCache().getChildren(path);
var liveTservers = children.map(TabletMetadata::checkServer).filter(Optional::isPresent).collect(Collectors.toSet());
log(liveTservers);
return liveTservers;
```
This requires `checkServer` to be modified to return either `Optional.of(instance)` or `Optional.empty()`, but I think it's more readable.
----------------------------------------------------------------
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] [accumulo] ctubbsii commented on a change in pull request #1821: Create listtablets shell command. Closes #1317
Posted by GitBox <gi...@apache.org>.
ctubbsii commented on a change in pull request #1821:
URL: https://github.com/apache/accumulo/pull/1821#discussion_r542744799
##########
File path: core/src/main/java/org/apache/accumulo/core/metadata/schema/TabletMetadata.java
##########
@@ -435,4 +445,34 @@ static TabletMetadata create(String id, String prevEndRow, String endRow) {
te.fetchedCols = EnumSet.of(ColumnType.PREV_ROW);
return te;
}
+
+ public static synchronized Set<TServerInstance> getLiveTServers(ClientContext context) {
+ final Set<TServerInstance> currentServers = new HashSet<>();
+
+ final String path = context.getZooKeeperRoot() + Constants.ZTSERVERS;
+
+ HashSet<String> all = new HashSet<>(context.getZooCache().getChildren(path));
+
+ for (String zPath : all) {
+ checkServer(context, currentServers, path, zPath);
+ }
Review comment:
The main point I find difficult to read is the pass by reference of the data structure to a different method that is responsible for updating the data structure, conditionally. That kind of pattern can be hard to follow.
So, instead of:
```java
methodToUpdateDataStructure(x1, x2, x3, dataStructure)
```
You can do:
```java
var obj = shouldAddToDataStructure(x1, x2, x3);
if (obj != null)
dataStructure.add(obj);
```
So, if the part of my suggestion that was less readable to you was the stream stuff, here's a non-stream version:
```suggestion
var currentServers = new HashSet<TServerInstance>();
for (var child : context.getZooCache().getChildren(context.getZooKeeperRoot() + Constants.ZTSERVERS)) {
var instance = checkServer(child);
if (instance != null)
currentServers.add(instance);
}
```
I just thought the stream version was better, but honestly, anything is better than the pass-by-reference mutable data structure pattern, IMO.
If you prefer the current implementation, that's fine too. I don't feel very strongly about it. It was just something I noticed and thought could be improved.
----------------------------------------------------------------
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] [accumulo] ctubbsii commented on a change in pull request #1821: Create listtablets shell command. Closes #1317
Posted by GitBox <gi...@apache.org>.
ctubbsii commented on a change in pull request #1821:
URL: https://github.com/apache/accumulo/pull/1821#discussion_r542744799
##########
File path: core/src/main/java/org/apache/accumulo/core/metadata/schema/TabletMetadata.java
##########
@@ -435,4 +445,34 @@ static TabletMetadata create(String id, String prevEndRow, String endRow) {
te.fetchedCols = EnumSet.of(ColumnType.PREV_ROW);
return te;
}
+
+ public static synchronized Set<TServerInstance> getLiveTServers(ClientContext context) {
+ final Set<TServerInstance> currentServers = new HashSet<>();
+
+ final String path = context.getZooKeeperRoot() + Constants.ZTSERVERS;
+
+ HashSet<String> all = new HashSet<>(context.getZooCache().getChildren(path));
+
+ for (String zPath : all) {
+ checkServer(context, currentServers, path, zPath);
+ }
Review comment:
The main point I find difficult to read is the pass by reference of the data structure to a different method that is responsible for updating the data structure, conditionally. That kind of pattern can be hard to follow.
So, instead of:
```java
methodToUpdateDataStructure(x1, x2, x3, dataStructure)
```
You can do:
```java
var obj = shouldAddToDataStructure(x1, x2, x3);
if (obj != null)
dataStructure.add(obj);
```
So, if the part of my suggestion that was less readable to you was the stream stuff, here's a non-stream version:
```java
var currentServers = new HashSet<TServerInstance>();
for (var child : context.getZooCache().getChildren(context.getZooKeeperRoot() + Constants.ZTSERVERS)) {
var instance = checkServer(child);
if (instance != null)
currentServers.add(instance);
}
```
I just thought the stream version was better, but honestly, anything is better than the pass-by-reference mutable data structure pattern, IMO.
If you prefer the current implementation, that's fine too. I don't feel very strongly about it. It was just something I noticed and thought could be improved.
----------------------------------------------------------------
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] [accumulo] milleruntime commented on a change in pull request #1821: Create listtablets shell command. Closes #1317
Posted by GitBox <gi...@apache.org>.
milleruntime commented on a change in pull request #1821:
URL: https://github.com/apache/accumulo/pull/1821#discussion_r543570402
##########
File path: core/src/main/java/org/apache/accumulo/core/metadata/schema/TabletMetadata.java
##########
@@ -435,4 +445,34 @@ static TabletMetadata create(String id, String prevEndRow, String endRow) {
te.fetchedCols = EnumSet.of(ColumnType.PREV_ROW);
return te;
}
+
+ public static synchronized Set<TServerInstance> getLiveTServers(ClientContext context) {
+ final Set<TServerInstance> currentServers = new HashSet<>();
+
+ final String path = context.getZooKeeperRoot() + Constants.ZTSERVERS;
+
+ HashSet<String> all = new HashSet<>(context.getZooCache().getChildren(path));
+
+ for (String zPath : all) {
+ checkServer(context, currentServers, path, zPath);
+ }
Review comment:
@ctubbsii I think I came up with a nice combo of ideas in ba8fb27
----------------------------------------------------------------
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] [accumulo] milleruntime commented on a change in pull request #1821: Create listtablets shell command. Closes #1317
Posted by GitBox <gi...@apache.org>.
milleruntime commented on a change in pull request #1821:
URL: https://github.com/apache/accumulo/pull/1821#discussion_r542476170
##########
File path: core/src/main/java/org/apache/accumulo/core/metadata/schema/TabletMetadata.java
##########
@@ -435,4 +445,34 @@ static TabletMetadata create(String id, String prevEndRow, String endRow) {
te.fetchedCols = EnumSet.of(ColumnType.PREV_ROW);
return te;
}
+
+ public static synchronized Set<TServerInstance> getLiveTServers(ClientContext context) {
+ final Set<TServerInstance> currentServers = new HashSet<>();
+
+ final String path = context.getZooKeeperRoot() + Constants.ZTSERVERS;
+
+ HashSet<String> all = new HashSet<>(context.getZooCache().getChildren(path));
+
+ for (String zPath : all) {
+ checkServer(context, currentServers, path, zPath);
+ }
Review comment:
I disagree. To me what you have is longer and less readable. I will rename all to children 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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [accumulo] milleruntime commented on a change in pull request #1821: Create listtablets shell command. Closes #1317
Posted by GitBox <gi...@apache.org>.
milleruntime commented on a change in pull request #1821:
URL: https://github.com/apache/accumulo/pull/1821#discussion_r542709205
##########
File path: test/src/main/java/org/apache/accumulo/test/functional/TabletMetadataIT.java
##########
@@ -0,0 +1,94 @@
+/*
+ * 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.accumulo.test.functional;
+
+import static org.apache.accumulo.fate.util.UtilWaitThread.sleepUninterruptibly;
+import static org.apache.accumulo.minicluster.ServerType.TABLET_SERVER;
+import static org.junit.Assert.assertEquals;
+
+import java.util.Set;
+import java.util.concurrent.TimeUnit;
+
+import org.apache.accumulo.core.client.Accumulo;
+import org.apache.accumulo.core.client.AccumuloClient;
+import org.apache.accumulo.core.clientImpl.ClientContext;
+import org.apache.accumulo.core.metadata.TServerInstance;
+import org.apache.accumulo.core.metadata.schema.TabletMetadata;
+import org.apache.accumulo.harness.MiniClusterConfigurationCallback;
+import org.apache.accumulo.harness.SharedMiniClusterBase;
+import org.apache.accumulo.miniclusterImpl.MiniAccumuloConfigImpl;
+import org.apache.hadoop.conf.Configuration;
+import org.junit.AfterClass;
+import org.junit.BeforeClass;
+import org.junit.Test;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+/**
+ * Tests features of the Ample TabletMetadata class that can't be tested in TabletMetadataTest
+ */
+public class TabletMetadataIT extends SharedMiniClusterBase {
+ private static final Logger log = LoggerFactory.getLogger(TabletMetadataIT.class);
+ private static final int NUM_TSERVERS = 4;
+
+ @BeforeClass
+ public static void setup() throws Exception {
+ SharedMiniClusterBase.startMiniClusterWithConfig(new Callback());
+ }
+
+ @AfterClass
+ public static void teardown() {
+ SharedMiniClusterBase.stopMiniCluster();
+ }
+
+ @Override
+ protected int defaultTimeoutSeconds() {
+ return 120;
+ }
+
+ private static class Callback implements MiniClusterConfigurationCallback {
+ @Override
+ public void configureMiniCluster(MiniAccumuloConfigImpl cfg, Configuration conf) {
+ cfg.setNumTservers(NUM_TSERVERS);
+ }
+ }
Review comment:
I don't know... despite having figured it out multiple times before, I still never know which IT to use... SharedMiniClusterBase, ConfigurableMacBase or AccumuloClusterHarness. I just no not to use AccumuloITBase if you want a MiniCluster.
----------------------------------------------------------------
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] [accumulo] milleruntime commented on a change in pull request #1821: Create listtablets shell command. Closes #1317
Posted by GitBox <gi...@apache.org>.
milleruntime commented on a change in pull request #1821:
URL: https://github.com/apache/accumulo/pull/1821#discussion_r543571190
##########
File path: test/src/main/java/org/apache/accumulo/test/functional/TabletMetadataIT.java
##########
@@ -0,0 +1,94 @@
+/*
+ * 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.accumulo.test.functional;
+
+import static org.apache.accumulo.fate.util.UtilWaitThread.sleepUninterruptibly;
+import static org.apache.accumulo.minicluster.ServerType.TABLET_SERVER;
+import static org.junit.Assert.assertEquals;
+
+import java.util.Set;
+import java.util.concurrent.TimeUnit;
+
+import org.apache.accumulo.core.client.Accumulo;
+import org.apache.accumulo.core.client.AccumuloClient;
+import org.apache.accumulo.core.clientImpl.ClientContext;
+import org.apache.accumulo.core.metadata.TServerInstance;
+import org.apache.accumulo.core.metadata.schema.TabletMetadata;
+import org.apache.accumulo.harness.MiniClusterConfigurationCallback;
+import org.apache.accumulo.harness.SharedMiniClusterBase;
+import org.apache.accumulo.miniclusterImpl.MiniAccumuloConfigImpl;
+import org.apache.hadoop.conf.Configuration;
+import org.junit.AfterClass;
+import org.junit.BeforeClass;
+import org.junit.Test;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+/**
+ * Tests features of the Ample TabletMetadata class that can't be tested in TabletMetadataTest
+ */
+public class TabletMetadataIT extends SharedMiniClusterBase {
+ private static final Logger log = LoggerFactory.getLogger(TabletMetadataIT.class);
+ private static final int NUM_TSERVERS = 4;
+
+ @BeforeClass
+ public static void setup() throws Exception {
+ SharedMiniClusterBase.startMiniClusterWithConfig(new Callback());
+ }
+
+ @AfterClass
+ public static void teardown() {
+ SharedMiniClusterBase.stopMiniCluster();
+ }
+
+ @Override
+ protected int defaultTimeoutSeconds() {
+ return 120;
+ }
+
+ private static class Callback implements MiniClusterConfigurationCallback {
+ @Override
+ public void configureMiniCluster(MiniAccumuloConfigImpl cfg, Configuration conf) {
+ cfg.setNumTservers(NUM_TSERVERS);
+ }
+ }
Review comment:
This made me think there are some ITs that could save on some code and use ConfigurableMacBase as well. I will do that as a follow on.
----------------------------------------------------------------
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] [accumulo] milleruntime merged pull request #1821: Create listtablets shell command. Closes #1317
Posted by GitBox <gi...@apache.org>.
milleruntime merged pull request #1821:
URL: https://github.com/apache/accumulo/pull/1821
----------------------------------------------------------------
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] [accumulo] ctubbsii commented on a change in pull request #1821: Create listtablets shell command. Closes #1317
Posted by GitBox <gi...@apache.org>.
ctubbsii commented on a change in pull request #1821:
URL: https://github.com/apache/accumulo/pull/1821#discussion_r542762909
##########
File path: test/src/main/java/org/apache/accumulo/test/functional/TabletMetadataIT.java
##########
@@ -0,0 +1,94 @@
+/*
+ * 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.accumulo.test.functional;
+
+import static org.apache.accumulo.fate.util.UtilWaitThread.sleepUninterruptibly;
+import static org.apache.accumulo.minicluster.ServerType.TABLET_SERVER;
+import static org.junit.Assert.assertEquals;
+
+import java.util.Set;
+import java.util.concurrent.TimeUnit;
+
+import org.apache.accumulo.core.client.Accumulo;
+import org.apache.accumulo.core.client.AccumuloClient;
+import org.apache.accumulo.core.clientImpl.ClientContext;
+import org.apache.accumulo.core.metadata.TServerInstance;
+import org.apache.accumulo.core.metadata.schema.TabletMetadata;
+import org.apache.accumulo.harness.MiniClusterConfigurationCallback;
+import org.apache.accumulo.harness.SharedMiniClusterBase;
+import org.apache.accumulo.miniclusterImpl.MiniAccumuloConfigImpl;
+import org.apache.hadoop.conf.Configuration;
+import org.junit.AfterClass;
+import org.junit.BeforeClass;
+import org.junit.Test;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+/**
+ * Tests features of the Ample TabletMetadata class that can't be tested in TabletMetadataTest
+ */
+public class TabletMetadataIT extends SharedMiniClusterBase {
+ private static final Logger log = LoggerFactory.getLogger(TabletMetadataIT.class);
+ private static final int NUM_TSERVERS = 4;
+
+ @BeforeClass
+ public static void setup() throws Exception {
+ SharedMiniClusterBase.startMiniClusterWithConfig(new Callback());
+ }
+
+ @AfterClass
+ public static void teardown() {
+ SharedMiniClusterBase.stopMiniCluster();
+ }
+
+ @Override
+ protected int defaultTimeoutSeconds() {
+ return 120;
+ }
+
+ private static class Callback implements MiniClusterConfigurationCallback {
+ @Override
+ public void configureMiniCluster(MiniAccumuloConfigImpl cfg, Configuration conf) {
+ cfg.setNumTservers(NUM_TSERVERS);
+ }
+ }
Review comment:
I always have to look at existing classes to see what they are doing, and then I look at the type-hierarchy just to be sure. I think ConfigurableMacBase is appropriate here.
----------------------------------------------------------------
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] [accumulo] milleruntime commented on a change in pull request #1821: Create listtablets shell command. Closes #1317
Posted by GitBox <gi...@apache.org>.
milleruntime commented on a change in pull request #1821:
URL: https://github.com/apache/accumulo/pull/1821#discussion_r551562134
##########
File path: test/src/main/java/org/apache/accumulo/test/functional/TabletMetadataIT.java
##########
@@ -0,0 +1,94 @@
+/*
+ * 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.accumulo.test.functional;
+
+import static org.apache.accumulo.fate.util.UtilWaitThread.sleepUninterruptibly;
+import static org.apache.accumulo.minicluster.ServerType.TABLET_SERVER;
+import static org.junit.Assert.assertEquals;
+
+import java.util.Set;
+import java.util.concurrent.TimeUnit;
+
+import org.apache.accumulo.core.client.Accumulo;
+import org.apache.accumulo.core.client.AccumuloClient;
+import org.apache.accumulo.core.clientImpl.ClientContext;
+import org.apache.accumulo.core.metadata.TServerInstance;
+import org.apache.accumulo.core.metadata.schema.TabletMetadata;
+import org.apache.accumulo.harness.MiniClusterConfigurationCallback;
+import org.apache.accumulo.harness.SharedMiniClusterBase;
+import org.apache.accumulo.miniclusterImpl.MiniAccumuloConfigImpl;
+import org.apache.hadoop.conf.Configuration;
+import org.junit.AfterClass;
+import org.junit.BeforeClass;
+import org.junit.Test;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+/**
+ * Tests features of the Ample TabletMetadata class that can't be tested in TabletMetadataTest
+ */
+public class TabletMetadataIT extends SharedMiniClusterBase {
+ private static final Logger log = LoggerFactory.getLogger(TabletMetadataIT.class);
+ private static final int NUM_TSERVERS = 4;
+
+ @BeforeClass
+ public static void setup() throws Exception {
+ SharedMiniClusterBase.startMiniClusterWithConfig(new Callback());
+ }
+
+ @AfterClass
+ public static void teardown() {
+ SharedMiniClusterBase.stopMiniCluster();
+ }
+
+ @Override
+ protected int defaultTimeoutSeconds() {
+ return 120;
+ }
+
+ private static class Callback implements MiniClusterConfigurationCallback {
+ @Override
+ public void configureMiniCluster(MiniAccumuloConfigImpl cfg, Configuration conf) {
+ cfg.setNumTservers(NUM_TSERVERS);
+ }
+ }
Review comment:
So in another branch, I modified BulkNewIT to use ConfigurableMacBase (it is currently using SharedMiniClusterBase) and the test went from being around 30 seconds long to over 4 minutes. I am not sure of the drastic difference between the two base classes but it seemed odd to me.
----------------------------------------------------------------
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