[jira] [Resolved] (ACCUMULO-3376) VolumeChooser API: replace String[] with Set
[ https://issues.apache.org/jira/browse/ACCUMULO-3376?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ] Christopher Tubbs resolved ACCUMULO-3376. - Resolution: Duplicate Closing as duplicate of https://github.com/apache/accumulo/pull/1551 > VolumeChooser API: replace String[] with Set > > > Key: ACCUMULO-3376 > URL: https://issues.apache.org/jira/browse/ACCUMULO-3376 > Project: Accumulo > Issue Type: Improvement > Components: tserver >Reporter: Christopher Tubbs >Priority: Major > > ACCUMULO-2061 added a Volume object. The VolumeChooser API and related volume > management code should utilize a set of these, rather than an array of String > objects. > This would be a minor improvement to what was done in ACCUMULO-3181, and > would avoid reintroducing invalid assumptions about the VolumeChooser API > semantics by avoiding the String type to represent volumes. -- This message was sent by Atlassian Jira (v8.3.4#803005)
[jira] [Resolved] (ACCUMULO-3411) Refactor VolumeManager initialization so a ServerConfigurationFactory is available
[ https://issues.apache.org/jira/browse/ACCUMULO-3411?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ] Christopher Tubbs resolved ACCUMULO-3411. - Resolution: Not A Problem This is OBE. VolumeChoosers now have an env for stuff like this. > Refactor VolumeManager initialization so a ServerConfigurationFactory is > available > -- > > Key: ACCUMULO-3411 > URL: https://issues.apache.org/jira/browse/ACCUMULO-3411 > Project: Accumulo > Issue Type: Improvement >Affects Versions: 1.7.0 >Reporter: Sean Busbey >Priority: Minor > > VolumeChooser would be improved by an init call that provided a > ServerConfigurationFactory. Currently, instances that need table > configuration have to instantiate their own lazily. > The initialization must be lazy because making their own instance requires an > Instance object. instantiating an HDFSZooInstance requires a VolumeManager. > Since the VolumeManager relies on their being a chooser, we get into a > circular dependency. > Either refactor VolumeManager instantiation so there is a > ServerConfigurationFactory that can be passed to the VolumeChooser or do > bootstrapping with a VolumeManager that doesn't use a VC to break the cycle. -- This message was sent by Atlassian Jira (v8.3.4#803005)
[GitHub] [accumulo] ctubbsii commented on issue #1551: Use sets instead of arrays for volumes
ctubbsii commented on issue #1551: Use sets instead of arrays for volumes URL: https://github.com/apache/accumulo/pull/1551#issuecomment-595585118 I wanted to knock this out because it will make a lot of the code cleanup of #1397 easier, I think, and I didn't want the pull request for that to get too big. 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: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [accumulo] ctubbsii opened a new pull request #1551: Use sets instead of arrays for volumes
ctubbsii opened a new pull request #1551: Use sets instead of arrays for volumes URL: https://github.com/apache/accumulo/pull/1551 Originally ACCUMULO-3376, this change replaces the use of `String[]` with `Set` throughout the VolumeChooser and VolumeManager code. This makes the API for VolumeChooser a bit more friendly, making it easier to use Streams and lambdas to filter and transform volumes throughout the code. This change also adds some more sanity checks on the instance volumes configuration (checking for empty list and duplicates). This also includes a utility to warn about planned API changes in internal pluggable code (such as the VolumeChooser), but without being too spammy. Update the default `VolumeChooser.choosable()` to return the full set of volumes, so that all are checked for flush and sync support on tserver startup. This choosable mechanism is a bit dubious anyway, because choosers could make different decisions over time (in response to configuration changes, for example) than they do at startup. So, defaulting to checking for the maximal set seems like a better strategy out-of-the-box. Make ServerConstants.baseUris unmodifiable. 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: us...@infra.apache.org With regards, Apache Git Services
[jira] [Resolved] (ACCUMULO-3794) Replication randomwalk module failed on verification
[
https://issues.apache.org/jira/browse/ACCUMULO-3794?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]
Christopher Tubbs resolved ACCUMULO-3794.
-
Resolution: Incomplete
This bug was reported several years ago, and does not appear to have been
actively pursued by anybody. It was closed as "Incomplete" to clean up old JIRA
issues after migrating issues to GitHub.
If anybody is interested in working on this, please create a new issue at our
current issue tracker at https://github.com/apache/accumulo/issues
> Replication randomwalk module failed on verification
>
>
> Key: ACCUMULO-3794
> URL: https://issues.apache.org/jira/browse/ACCUMULO-3794
> Project: Accumulo
> Issue Type: Bug
> Components: replication, test
>Reporter: Josh Elser
>Assignee: Eric C. Newton
>Priority: Major
> Labels: 1.7.0_QA
>
> {noformat}
> 11 03:37:45,852 [randomwalk.Framework] ERROR: Error during random walk
> java.lang.Exception: Error running node Concurrent.xml
> at org.apache.accumulo.test.randomwalk.Module.visit(Module.java:346)
> at
> org.apache.accumulo.test.randomwalk.Framework.run(Framework.java:59)
> at
> org.apache.accumulo.test.randomwalk.Framework.main(Framework.java:119)
> at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
> at
> sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:57)
> at
> sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
> at java.lang.reflect.Method.invoke(Method.java:606)
> at org.apache.accumulo.start.Main$2.run(Main.java:130)
> at java.lang.Thread.run(Thread.java:745)
> Caused by: java.lang.Exception: Error running node ct.Replication
> at org.apache.accumulo.test.randomwalk.Module.visit(Module.java:346)
> at org.apache.accumulo.test.randomwalk.Module$1.call(Module.java:283)
> at org.apache.accumulo.test.randomwalk.Module$1.call(Module.java:278)
> at java.util.concurrent.FutureTask.run(FutureTask.java:262)
> at
> java.util.concurrent.Executors$RunnableAdapter.call(Executors.java:471)
> at java.util.concurrent.FutureTask.run(FutureTask.java:262)
> at
> java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1145)
> at
> java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:615)
> at
> org.apache.accumulo.fate.util.LoggingRunnable.run(LoggingRunnable.java:35)
> ... 1 more
> Caused by: java.lang.RuntimeException: 0 fails to match expected value 1000
> at
> org.apache.accumulo.test.randomwalk.concurrent.Replication.assertEquals(Replication.java:180)
> at
> org.apache.accumulo.test.randomwalk.concurrent.Replication.visit(Replication.java:167)
> ... 9 more
> {noformat}
> Leading up to this, the test was waiting on a WAL to be replicated
> {noformat}
> 11 03:37:15,334 [impl.ReplicationOperationsImpl] DEBUG: Collecting referenced
> files for replication of table
> repl_source_026fb5bb_b94c_4a7b_8480_d649d7b383f5
> 11 03:37:15,334 [impl.ReplicationOperationsImpl] DEBUG: Found id of 51 for
> name repl_source_026fb5bb_b94c_4a7b_8480_d649d7b383f5
> 11 03:37:15,345 [concurrent.Replication] DEBUG: updateFileRefs size 1
> 11 03:37:15,707 [impl.ReplicationOperationsImpl] DEBUG: Collecting referenced
> files for replication of table
> repl_source_026fb5bb_b94c_4a7b_8480_d649d7b383f5
> 11 03:37:15,707 [impl.ReplicationOperationsImpl] DEBUG: Found id of 51 for
> name repl_source_026fb5bb_b94c_4a7b_8480_d649d7b383f5
> 11 03:37:15,713 [concurrent.Replication] DEBUG: updateFileRefs size 0
> {noformat}
> It would seem that we waited on a WAL correctly, but the WAL we waiting on
> didn't contain the records we were waiting for (or there's a bug elsewhere).
--
This message was sent by Atlassian Jira
(v8.3.4#803005)
[jira] [Resolved] (ACCUMULO-4389) ReplicationOperations().drain(..) may return too quickly
[
https://issues.apache.org/jira/browse/ACCUMULO-4389?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]
Christopher Tubbs resolved ACCUMULO-4389.
-
Resolution: Incomplete
This bug was reported several years ago, and does not appear to have been
actively pursued by anybody. It was closed as "Incomplete" to clean up old JIRA
issues after migrating issues to GitHub.
If anybody is interested in working on this, please create a new issue at our
current issue tracker at https://github.com/apache/accumulo/issues
> ReplicationOperations().drain(..) may return too quickly
>
>
> Key: ACCUMULO-4389
> URL: https://issues.apache.org/jira/browse/ACCUMULO-4389
> Project: Accumulo
> Issue Type: Bug
> Components: replication
>Reporter: Josh Elser
>Assignee: Josh Elser
>Priority: Critical
>
> Was taking a look at some logs from automated tests that [~romil.choksi] sent
> my way and noticed that MultiInstanceReplicationIT was failing infrequently.
> Looking at the output, I can see that the call was returning very quickly
> (essentially in the amount of time the RPC would take on the slow test
> hardware)
> {noformat}
> Drain completed in 25ms
> {noformat}
> Looking at the implementation of
> {{MasterClientServiceHandler.drainReplicationTable(...)}}, it's not handling
> the references we read from the metadata table correctly. I believe this is
> causing the test to return too quickly.
--
This message was sent by Atlassian Jira
(v8.3.4#803005)
[jira] [Resolved] (ACCUMULO-4485) Missing entries and dying tservers follow replication with fault injection on source cluster
[
https://issues.apache.org/jira/browse/ACCUMULO-4485?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]
Christopher Tubbs resolved ACCUMULO-4485.
-
Resolution: Incomplete
This bug was reported several years ago, and does not appear to have been
actively pursued by anybody. It was closed as "Incomplete" to clean up old JIRA
issues after migrating issues to GitHub.
If anybody is interested in working on this, please create a new issue at our
current issue tracker at https://github.com/apache/accumulo/issues
> Missing entries and dying tservers follow replication with fault injection on
> source cluster
>
>
> Key: ACCUMULO-4485
> URL: https://issues.apache.org/jira/browse/ACCUMULO-4485
> Project: Accumulo
> Issue Type: Bug
> Components: replication
>Affects Versions: 1.7.2
>Reporter: Dima Spivak
>Priority: Critical
>
> In running some of the [existing testing around
> replication|https://github.com/apache/accumulo/tree/master/test/system/merkle-replication]
> (you da man, [~elserj]), I've hit what looks like a serious data loss
> problem.
> To provide some background: I'm testing Accumulo 1.7.2 using 2 6-node
> clusterdock-based clusters (source cluster nodes are named {{node-1.cluster}}
> through to {{node-6.cluster}} and destination cluster nodes are named
> {{node-7.cluster}} through {{node-12.cluster}}). After starting up the
> clusters, I first established that the test itself works (and, again, it's
> really nifty) by modifying {{merkle-env.sh}} to point to the correct ZK and
> Accumulo instances and setting {{NUM_RECORDS=1}} to shorten run times
> by an order of magnitude. I then run {{configure-replication.sh}},
> {{ingest-data.sh}}, and then when the ingest script returns (and the UI shows
> ingest counts down to 0), I restart the source cluster to ensure WALs are
> available to replicate. I then wait and see that the replication UI on the
> source cluster says it has no more files to replicate (and that ingest on the
> destination cluster is also complete), and then run {{verify-data.sh}} to
> confirm that all went well. This goes off without a hitch and I've run it a
> dozen times without a single failure. Good.
> Now, just to throw a spanner in the works, I enable fault-injection on the
> source cluster while the data is being written with {{ingest-data.sh}} and
> after restarting the Accumulo cluster on the source cluster. Because I'm
> running testing on clusters that use Cloudera Manager, I use an in-house
> fault injection tool that allows me to specify policies for killing any
> cluster services or roles using the CM REST API. In my case, I simply have it
> kill datanodes, the Accumulo master, tservers, tracers, and monitor at random
> intervals. Starting this up and then starting the data ingest doesn't seem to
> be a problem and my source cluster eventually gets all its data written and
> the script returns (and, it's worth noting, I get the same root hash for this
> source cluster across repeated runs). On the destination cluster, though,
> things don't seem to be behaving. In particular, I've observed that tservers
> appear to die with log messages ending with this ([full logs on
> Gist|https://gist.github.com/dimaspivak/2200ab10e0e00a4a7b81a5f6d83e3d85]):
> {code}
> 2016-10-04 15:46:54,458 [replication.BatchWriterReplicationReplayer] INFO :
> Applying 11048 mutations to table replicationDestination as part of batch
> 2016-10-04 15:46:55,473 [zookeeper.ClientCnxn] WARN : Session
> 0x157904c6753001c for server node-7.cluster/192.168.124.8:2181, unexpected
> error, closing socket connection and attempting reconnect
> java.io.IOException: Broken pipe
> at sun.nio.ch.FileDispatcherImpl.write0(Native Method)
> at sun.nio.ch.SocketDispatcher.write(SocketDispatcher.java:47)
> at sun.nio.ch.IOUtil.writeFromNativeBuffer(IOUtil.java:93)
> at sun.nio.ch.IOUtil.write(IOUtil.java:65)
> at sun.nio.ch.SocketChannelImpl.write(SocketChannelImpl.java:487)
> at
> org.apache.zookeeper.ClientCnxnSocketNIO.doIO(ClientCnxnSocketNIO.java:117)
> at
> org.apache.zookeeper.ClientCnxnSocketNIO.doTransport(ClientCnxnSocketNIO.java:355)
> at org.apache.zookeeper.ClientCnxn$SendThread.run(ClientCnxn.java:1081)
> 2016-10-04 15:46:55,573 [zookeeper.ZooReader] WARN : Saw (possibly) transient
> exception communicating with ZooKeeper
> org.apache.zookeeper.KeeperException$ConnectionLossException: KeeperErrorCode
> = ConnectionLoss for
> /accumulo/75b12d57-3962-4427-8de7-1586b7cf7be5/tservers/node-8.cluster:10011
> at org.apache.zookeeper.KeeperException.create(KeeperException.java:99)
> at org.apache.zookeeper.KeeperException.create(KeeperException.java:51)
>
[jira] [Resolved] (ACCUMULO-3310) Garbage collection might remove files with references in the replication table
[
https://issues.apache.org/jira/browse/ACCUMULO-3310?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]
Christopher Tubbs resolved ACCUMULO-3310.
-
Resolution: Incomplete
This bug was reported several years ago, and does not appear to have been
actively pursued by anybody. It was closed as "Incomplete" to clean up old JIRA
issues after migrating issues to GitHub.
If anybody is interested in working on this, please create a new issue at our
current issue tracker at https://github.com/apache/accumulo/issues
> Garbage collection might remove files with references in the replication table
> --
>
> Key: ACCUMULO-3310
> URL: https://issues.apache.org/jira/browse/ACCUMULO-3310
> Project: Accumulo
> Issue Type: Bug
> Components: replication
>Reporter: Christopher Tubbs
>Priority: Major
>
> There's a possible race condition deleting files from HDFS still needed for
> replication. The Accumulo garbage collector will not check for references to
> files in use if it thinks that the replication table does not exist (or is
> offline, as the case will be after ACCUMULO-3147 is completed). The problem
> with this is that the table may actually exist, and may be the only place
> where the in-use file is still referenced, but the garbage collector may not
> yet know the table exists.
> This probably has a very low probability of happening, and only really
> affects the case when replication is first started. However, if it did
> happen, it would mean an error replicating data.
> Some solutions discussed with [~elserj]:
> # Always scan the table. _This requires the table to always exist and be
> online and available for scanning._
> # Keep the status messages around until after they are removed from the
> replication table. This requires moving the deletes from {{StatusMaker}} to
> {{RemoveCompleteReplicationRecords}}. _There may be some implications for
> delaying removal of these entries, but probably not too many, and none we
> can't work through._
> # Move the contents of the replication table to the metadata table and don't
> have a separate replication table. _This would require a big refactoring, and
> might have performance implications to the metadata table._
--
This message was sent by Atlassian Jira
(v8.3.4#803005)
[jira] [Updated] (ACCUMULO-3794) Replication randomwalk module failed on verification
[
https://issues.apache.org/jira/browse/ACCUMULO-3794?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]
Christopher Tubbs updated ACCUMULO-3794:
Component/s: replication
> Replication randomwalk module failed on verification
>
>
> Key: ACCUMULO-3794
> URL: https://issues.apache.org/jira/browse/ACCUMULO-3794
> Project: Accumulo
> Issue Type: Bug
> Components: replication, test
>Reporter: Josh Elser
>Assignee: Eric C. Newton
>Priority: Major
> Labels: 1.7.0_QA
>
> {noformat}
> 11 03:37:45,852 [randomwalk.Framework] ERROR: Error during random walk
> java.lang.Exception: Error running node Concurrent.xml
> at org.apache.accumulo.test.randomwalk.Module.visit(Module.java:346)
> at
> org.apache.accumulo.test.randomwalk.Framework.run(Framework.java:59)
> at
> org.apache.accumulo.test.randomwalk.Framework.main(Framework.java:119)
> at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
> at
> sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:57)
> at
> sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
> at java.lang.reflect.Method.invoke(Method.java:606)
> at org.apache.accumulo.start.Main$2.run(Main.java:130)
> at java.lang.Thread.run(Thread.java:745)
> Caused by: java.lang.Exception: Error running node ct.Replication
> at org.apache.accumulo.test.randomwalk.Module.visit(Module.java:346)
> at org.apache.accumulo.test.randomwalk.Module$1.call(Module.java:283)
> at org.apache.accumulo.test.randomwalk.Module$1.call(Module.java:278)
> at java.util.concurrent.FutureTask.run(FutureTask.java:262)
> at
> java.util.concurrent.Executors$RunnableAdapter.call(Executors.java:471)
> at java.util.concurrent.FutureTask.run(FutureTask.java:262)
> at
> java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1145)
> at
> java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:615)
> at
> org.apache.accumulo.fate.util.LoggingRunnable.run(LoggingRunnable.java:35)
> ... 1 more
> Caused by: java.lang.RuntimeException: 0 fails to match expected value 1000
> at
> org.apache.accumulo.test.randomwalk.concurrent.Replication.assertEquals(Replication.java:180)
> at
> org.apache.accumulo.test.randomwalk.concurrent.Replication.visit(Replication.java:167)
> ... 9 more
> {noformat}
> Leading up to this, the test was waiting on a WAL to be replicated
> {noformat}
> 11 03:37:15,334 [impl.ReplicationOperationsImpl] DEBUG: Collecting referenced
> files for replication of table
> repl_source_026fb5bb_b94c_4a7b_8480_d649d7b383f5
> 11 03:37:15,334 [impl.ReplicationOperationsImpl] DEBUG: Found id of 51 for
> name repl_source_026fb5bb_b94c_4a7b_8480_d649d7b383f5
> 11 03:37:15,345 [concurrent.Replication] DEBUG: updateFileRefs size 1
> 11 03:37:15,707 [impl.ReplicationOperationsImpl] DEBUG: Collecting referenced
> files for replication of table
> repl_source_026fb5bb_b94c_4a7b_8480_d649d7b383f5
> 11 03:37:15,707 [impl.ReplicationOperationsImpl] DEBUG: Found id of 51 for
> name repl_source_026fb5bb_b94c_4a7b_8480_d649d7b383f5
> 11 03:37:15,713 [concurrent.Replication] DEBUG: updateFileRefs size 0
> {noformat}
> It would seem that we waited on a WAL correctly, but the WAL we waiting on
> didn't contain the records we were waiting for (or there's a bug elsewhere).
--
This message was sent by Atlassian Jira
(v8.3.4#803005)
[GitHub] [accumulo] keith-turner commented on issue #1549: Fixes #1542 - Created a randomIndex member of ThriftTransportPool
keith-turner commented on issue #1549: Fixes #1542 - Created a randomIndex member of ThriftTransportPool URL: https://github.com/apache/accumulo/pull/1549#issuecomment-595520860 > @keith-turner If you believe your fix is good, would you mind committing the change, or creating a PR to make it more easy to review? Sure I can do that tomorrow, the patch is a bit unwieldy. 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: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [accumulo] ctubbsii commented on issue #1549: Fixes #1542 - Created a randomIndex member of ThriftTransportPool
ctubbsii commented on issue #1549: Fixes #1542 - Created a randomIndex member of ThriftTransportPool URL: https://github.com/apache/accumulo/pull/1549#issuecomment-595516400 @keith-turner If you believe your fix is good, would you mind committing the change, or creating a PR to make it more easy to review? 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: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [accumulo] keith-turner commented on issue #1549: Fixes #1542 - Created a randomIndex member of ThriftTransportPool
keith-turner commented on issue #1549: Fixes #1542 - Created a randomIndex
member of ThriftTransportPool
URL: https://github.com/apache/accumulo/pull/1549#issuecomment-595492380
I think the goal of the fix is good. One way to achieve the goal without
changing ThriftTransportPool is to pass a list of one server to it. I tried
this locally using the List.subList function and ran the test successfully a
bunch of times. Also looking at the code that gets the list of servers I see
its incorrect (it never reads from zookeeper again) and way more complicated
than needed. I simplified it while experimenting. Below are the diffs from my
local testing.
```diff
--- a/test/src/main/java/org/apache/accumulo/test/TransportCachingIT.java
+++ b/test/src/main/java/org/apache/accumulo/test/TransportCachingIT.java
@@ -18,15 +18,12 @@
*/
package org.apache.accumulo.test;
-import static java.nio.charset.StandardCharsets.UTF_8;
import static org.junit.Assert.assertNotNull;
import static org.junit.Assert.assertNotSame;
import static org.junit.Assert.assertSame;
-import java.util.ArrayList;
import java.util.List;
-import org.apache.accumulo.core.Constants;
import org.apache.accumulo.core.client.Accumulo;
import org.apache.accumulo.core.client.AccumuloClient;
import org.apache.accumulo.core.clientImpl.ClientContext;
@@ -34,9 +31,7 @@ import
org.apache.accumulo.core.clientImpl.ThriftTransportKey;
import org.apache.accumulo.core.clientImpl.ThriftTransportPool;
import org.apache.accumulo.core.conf.ConfigurationTypeHelper;
import org.apache.accumulo.core.conf.Property;
-import org.apache.accumulo.core.util.ServerServices;
-import org.apache.accumulo.core.util.ServerServices.Service;
-import org.apache.accumulo.fate.zookeeper.ZooCache;
+import org.apache.accumulo.core.util.HostAndPort;
import org.apache.accumulo.harness.AccumuloClusterHarness;
import org.apache.thrift.transport.TTransport;
import org.apache.thrift.transport.TTransportException;
@@ -44,61 +39,32 @@ import org.junit.Test;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
+import com.google.common.collect.Lists;
+
/**
* Test that {@link ThriftTransportPool} actually adheres to the
cachedConnection argument
*/
public class TransportCachingIT extends AccumuloClusterHarness {
private static final Logger log =
LoggerFactory.getLogger(TransportCachingIT.class);
- private static int ATTEMPTS = 0;
@Test
public void testCachedTransport() throws InterruptedException {
try (AccumuloClient client =
Accumulo.newClient().from(getClientProps()).build()) {
- while (client.instanceOperations().getTabletServers().isEmpty()) {
+
+ List tservers;
+
+ while ((tservers =
client.instanceOperations().getTabletServers()).isEmpty()) {
// sleep until a tablet server is up
Thread.sleep(50);
}
+
ClientContext context = (ClientContext) client;
long rpcTimeout =
ConfigurationTypeHelper.getTimeInMillis(Property.GENERAL_RPC_TIMEOUT.getDefaultValue());
- ZooCache zc = context.getZooCache();
- final String zkRoot = context.getZooKeeperRoot();
-
- // wait until Zookeeper is populated
- List children = zc.getChildren(zkRoot + Constants.ZTSERVERS);
- while (children.isEmpty()) {
-Thread.sleep(100);
-children = zc.getChildren(zkRoot + Constants.ZTSERVERS);
- }
-
- ArrayList servers = new ArrayList<>();
- while (servers.isEmpty()) {
-for (String tserver : children) {
- String path = zkRoot + Constants.ZTSERVERS + "/" + tserver;
- byte[] data = zc.getLockData(path);
- if (data != null) {
-String strData = new String(data, UTF_8);
-if (!strData.equals("master"))
- servers.add(new ThriftTransportKey(
- new
ServerServices(strData).getAddress(Service.TSERV_CLIENT), rpcTimeout,
- context));
- }
-}
-ATTEMPTS++;
-if (!servers.isEmpty())
- break;
-else {
- if (ATTEMPTS < 100) {
-log.warn("Making another attempt to add ThriftTransportKey
servers");
-Thread.sleep(100);
- } else {
-log.error("Failed to add ThriftTransportKey servers - Failing
TransportCachingIT test");
-org.junit.Assert
-.fail("Failed to add ThriftTransportKey servers - Failing
TransportCachingIT test");
- }
-}
- }
+ List servers = Lists.transform(tservers,
+ serverStr -> new
ThriftTransportKey(HostAndPort.fromString(serverStr), rpcTimeout,
+ context));
[GitHub] [accumulo] ctubbsii opened a new pull request #1550: Remove some reflection where not needed
ctubbsii opened a new pull request #1550: Remove some reflection where not needed URL: https://github.com/apache/accumulo/pull/1550 Remove unnecessary reflection to load methods that are contained in public stable Hadoop APIs. This reflection was added for simultaneous Hadoop 1 and Hadoop 2 support. We now require Hadoop 3 and these APIs are stable, so reflection is not needed. Also reduce visibility of some methods in TTimeoutTransport exposed for testing and delete unnecessary code. 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: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [accumulo] ctubbsii commented on issue #1241: Misc ThriftTransportPool improvements
ctubbsii commented on issue #1241: Misc ThriftTransportPool improvements URL: https://github.com/apache/accumulo/pull/1241#issuecomment-595474455 > The goal was to avoid a linear search when removing idle connections. Thanks. That helps. 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: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [accumulo] ctubbsii commented on issue #1397: Transition from instance.dfs.{uri, dir} to instance.volumes on upgrade
ctubbsii commented on issue #1397: Transition from instance.dfs.{uri,dir} to
instance.volumes on upgrade
URL: https://github.com/apache/accumulo/issues/1397#issuecomment-595472758
I'll take a look at this one.
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:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [accumulo] keith-turner commented on issue #1453: Update Last Location. Solution for #1169
keith-turner commented on issue #1453: Update Last Location. Solution for #1169 URL: https://github.com/apache/accumulo/pull/1453#issuecomment-595449419 @Manno15 it would be good to make the change in a single metadata table mutation so that its atomic. I think `TabletStateStore. setLocations()` is only used by the tablet server. The code in MetaDataStateStore, RootTabletStateStore, and LoggingTabletStateStore are implementations of the interface. I would remove the method `TabletStateStore. setLocations()` and add a method `TabletStateStore. setLocation(newLoc, prevLastLoc)` then update the implementations and the calling code. 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: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [accumulo] milleruntime commented on issue #1397: Transition from instance.dfs.{uri, dir} to instance.volumes on upgrade
milleruntime commented on issue #1397: Transition from instance.dfs.{uri,dir}
to instance.volumes on upgrade
URL: https://github.com/apache/accumulo/issues/1397#issuecomment-595415142
Unassigned myself as I am currently not working on this. The replacement of
relative paths on upgrade and removal of relative path code is done. Removal
of the old properties still needs to be 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:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [accumulo] milleruntime commented on issue #1549: Fixes #1542 - Created a randomIndex member of ThriftTransportPool
milleruntime commented on issue #1549: Fixes #1542 - Created a randomIndex member of ThriftTransportPool URL: https://github.com/apache/accumulo/pull/1549#issuecomment-595406345 Thanks for looking at this test. While this might help with the randomness of the ThriftTransportPool, I am usually against adding logic to change the behavior of a class specifically for a test. I think the code [here](https://github.com/apache/accumulo/pull/1241/files#diff-a8fae6237f477626112730e174b19a78R144-R166) should be removed as I am not sure it is testing what it was intended to. I will defer to @keith-turner since he worked on that change. Also, I think this test should check to make sure the server list has both tservers in it before proceeding. Currently it will just grab the first tserver it sees in ZK, breaking out of the ```servers.isEmpty()``` loop before seeing the second one. 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: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [accumulo] jzgithub1 opened a new pull request #1549: Fixes #1542 - Created a randomIndex member of ThriftTransportPool
jzgithub1 opened a new pull request #1549: Fixes #1542 - Created a randomIndex member of ThriftTransportPool URL: https://github.com/apache/accumulo/pull/1549 for testing. When the preferredCachedConnection was false the randomness of the selection of the server index caused inconsistent results in test. The integrated test actually reveals something that needs to be corrected in the usage of ThriftTransportPool in general but I will let others decide that. This change allows the TransportCachingIT to run 30 times consecutively without failure. I haven't tried to go higher in count. 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: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [accumulo] Manno15 commented on issue #1453: Update Last Location. Solution for #1169
Manno15 commented on issue #1453: Update Last Location. Solution for #1169 URL: https://github.com/apache/accumulo/pull/1453#issuecomment-595389025 @keith-turner It appears that setLocation is used by several other files like MetaDataStateStore, RootTabletStateStore, and LoggingTabletStateStore. So I was thinking instead to separate that feature out into a setLastLocation function similar to how there is a current setLocations and setFutureLocations. Thoughts on that or should I modify each of the existing functions to handle a location parameter? 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: us...@infra.apache.org With regards, Apache Git Services
Accumulo-Master - Build # 3050 - Still Unstable
The Apache Jenkins build system has built Accumulo-Master (build #3050) Status: Still Unstable Check console output at https://builds.apache.org/job/Accumulo-Master/3050/ to view the results.
[GitHub] [accumulo] jzgithub1 commented on issue #1241: Misc ThriftTransportPool improvements
jzgithub1 commented on issue #1241: Misc ThriftTransportPool improvements URL: https://github.com/apache/accumulo/pull/1241#issuecomment-595347678 @ctubbsii and @keith-turner . I think you are getting to the root of the problem of the intermittent failure of the TransportCachingIT.testCachedTransport with this improvement (#1542). The flakiness of that test is showing a problem with the function ```java private TTransport getTransport(ThriftTransportKey cacheKey) ``` in my opinion. I will make comments on that issue also. 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: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [accumulo] milleruntime commented on issue #1548: Document transport pool unreserved queue
milleruntime commented on issue #1548: Document transport pool unreserved queue URL: https://github.com/apache/accumulo/pull/1548#issuecomment-595341246 > Related to these changes, tests were added to the TransportCachingIT [here](https://github.com/apache/accumulo/pull/1241/files#diff-a8fae6237f477626112730e174b19a78R144-R166) but I don't believe they are reliable tests or testing the behaviour you updated. The method called `getAnyTransport` in the IT can return a random entry of the server set, which may not be the one the test is expecting. Occasional failure can be seen here: https://github.com/apache/accumulo/issues/1542 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: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [accumulo] milleruntime commented on issue #1548: Document transport pool unreserved queue
milleruntime commented on issue #1548: Document transport pool unreserved queue URL: https://github.com/apache/accumulo/pull/1548#issuecomment-595340207 Related to these changes, tests were added to the TransportCachingIT [here](https://github.com/apache/accumulo/pull/1241/files#diff-a8fae6237f477626112730e174b19a78R144-R166) but I don't believe they are reliable tests or testing the behaviour you updated. The method called ```getAnyTransport``` in the IT can return a random entry of the server set, which may not be the one the test is expecting. 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: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [accumulo] keith-turner commented on issue #1241: Misc ThriftTransportPool improvements
keith-turner commented on issue #1241: Misc ThriftTransportPool improvements URL: https://github.com/apache/accumulo/pull/1241#issuecomment-595337205 It certainly could have had a better commit message. I can't remember the specifics of how this came to be. The goal was to avoid a linear search when removing idle connections. 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: us...@infra.apache.org With regards, Apache Git Services
Accumulo-Master - Build # 3049 - Still Unstable
The Apache Jenkins build system has built Accumulo-Master (build #3049) Status: Still Unstable Check console output at https://builds.apache.org/job/Accumulo-Master/3049/ to view the results.
[GitHub] [accumulo] keith-turner commented on issue #1548: Document transport pool unreserved queue
keith-turner commented on issue #1548: Document transport pool unreserved queue URL: https://github.com/apache/accumulo/pull/1548#issuecomment-595334883 The formatter does not mess with it. I had to put empty lines between the bullet points to keep the formatter from squishing them all into a paragraph. 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: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [accumulo] ctubbsii commented on issue #1241: Misc ThriftTransportPool improvements
ctubbsii commented on issue #1241: Misc ThriftTransportPool improvements URL: https://github.com/apache/accumulo/pull/1241#issuecomment-595334862 @keith-turner Thanks. #1548 looks good to me to document what the new code does. However, I was more asking about the changeset... something that referenced the change itself and the basic reason, as in "Use Deque instead of LinkedList for CachedConnections because X". I can't tell without a deeper dive, but I *think* there was no change in behavior, so X here would be something like "more efficient retrieval of recent connections". A one-liner something along those lines would be better than the current "Misc improvements" comment (although we can't change history, I thought it would be nice to document here for reference). 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: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [accumulo] keith-turner commented on issue #1241: Misc ThriftTransportPool improvements
keith-turner commented on issue #1241: Misc ThriftTransportPool improvements URL: https://github.com/apache/accumulo/pull/1241#issuecomment-595322748 @ctubbsii I opened #1548 to add documentation to the code. I also modified the code to use methods present in Dequeue that are more descriptive. 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: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [accumulo] keith-turner opened a new pull request #1548: Document transport pool unreserved queue
keith-turner opened a new pull request #1548: Document transport pool unreserved queue URL: https://github.com/apache/accumulo/pull/1548 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: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [accumulo] ctubbsii commented on issue #1546: Replace try/finally with try-with-resources.
ctubbsii commented on issue #1546: Replace try/finally with try-with-resources. URL: https://github.com/apache/accumulo/pull/1546#issuecomment-595317766 @jmark99 I went ahead and merged what you had, since it was fine. Any `var` improvements can be done as follow-on... if you even want to (it's not really necessary). 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: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [accumulo] ctubbsii merged pull request #1546: Replace try/finally with try-with-resources.
ctubbsii merged pull request #1546: Replace try/finally with try-with-resources. URL: https://github.com/apache/accumulo/pull/1546 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: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [accumulo] ctubbsii merged pull request #1545: Remove miniDFS from HalfDeadTServerIT
ctubbsii merged pull request #1545: Remove miniDFS from HalfDeadTServerIT URL: https://github.com/apache/accumulo/pull/1545 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: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [accumulo] ctubbsii commented on a change in pull request #1545: Remove miniDFS from HalfDeadTServerIT
ctubbsii commented on a change in pull request #1545: Remove miniDFS from
HalfDeadTServerIT
URL: https://github.com/apache/accumulo/pull/1545#discussion_r388352906
##
File path:
test/src/main/java/org/apache/accumulo/test/functional/HalfDeadTServerIT.java
##
@@ -63,7 +66,29 @@ protected int defaultTimeoutSeconds() {
return 4 * 60;
}
- class DumpOutput extends Daemon {
+ private static final AtomicBoolean sharedLibBuilt = new AtomicBoolean(false);
+
+ @SuppressFBWarnings(value = "COMMAND_INJECTION",
+ justification = "command executed is not from user input")
+ @BeforeClass
+ public static void buildSharedLib() throws IOException, InterruptedException
{
+String root = System.getProperty("user.dir");
+String source = root + "/src/test/c/fake_disk_failure.c";
+String lib = root + "/target/fake_disk_failure.so";
Review comment:
Perhaps not, but that's a bit out of scope of what I was trying to optimize
here. I was actually thinking about creating a proper Makefile and building the
shared lib with exec-maven-plugin rather than doing it with a direct call to
gcc in the test. But again, I felt it was a bit out of scope.
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:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [accumulo] ctubbsii commented on a change in pull request #1546: Replace try/finally with try-with-resources.
ctubbsii commented on a change in pull request #1546: Replace try/finally with
try-with-resources.
URL: https://github.com/apache/accumulo/pull/1546#discussion_r388351313
##
File path:
core/src/main/java/org/apache/accumulo/core/file/rfile/bcfile/PrintInfo.java
##
@@ -39,10 +39,8 @@
public static void printMetaBlockInfo(SiteConfiguration siteConfig,
Configuration conf,
FileSystem fs, Path path) throws IOException {
FSDataInputStream fsin = fs.open(path);
-BCFile.Reader bcfr = null;
-try {
- bcfr = new BCFile.Reader(fsin, fs.getFileStatus(path).getLen(), conf,
- CryptoServiceFactory.newInstance(siteConfig,
ClassloaderType.ACCUMULO));
+try (BCFile.Reader bcfr = new BCFile.Reader(fsin,
fs.getFileStatus(path).getLen(), conf,
Review comment:
That's fine. We can merge this without those suggestions. They aren't
necessary.
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:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [accumulo] ctubbsii commented on issue #1241: Misc ThriftTransportPool improvements
ctubbsii commented on issue #1241: Misc ThriftTransportPool improvements URL: https://github.com/apache/accumulo/pull/1241#issuecomment-595275496 @keith-turner This issue never really had any description of what these improvements were intended to achieve. Can you briefly summarize here what the overall intent was, to give some context? 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: us...@infra.apache.org With regards, Apache Git Services
Accumulo-Master - Build # 3048 - Still Unstable
The Apache Jenkins build system has built Accumulo-Master (build #3048) Status: Still Unstable Check console output at https://builds.apache.org/job/Accumulo-Master/3048/ to view the results.
[GitHub] [accumulo] milleruntime merged pull request #1547: Drop obscuring overrides in thrift client code
milleruntime merged pull request #1547: Drop obscuring overrides in thrift client code URL: https://github.com/apache/accumulo/pull/1547 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: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [accumulo] milleruntime commented on issue #1547: Drop obscuring overrides in thrift client code
milleruntime commented on issue #1547: Drop obscuring overrides in thrift client code URL: https://github.com/apache/accumulo/pull/1547#issuecomment-595233611 I ran "mvn clean verify javadoc:jar -DskipITs" locally and it built successfully. 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: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [accumulo] jmark99 commented on a change in pull request #1546: Replace try/finally with try-with-resources.
jmark99 commented on a change in pull request #1546: Replace try/finally with
try-with-resources.
URL: https://github.com/apache/accumulo/pull/1546#discussion_r388272607
##
File path:
core/src/main/java/org/apache/accumulo/core/file/rfile/bcfile/PrintInfo.java
##
@@ -39,10 +39,8 @@
public static void printMetaBlockInfo(SiteConfiguration siteConfig,
Configuration conf,
FileSystem fs, Path path) throws IOException {
FSDataInputStream fsin = fs.open(path);
-BCFile.Reader bcfr = null;
-try {
- bcfr = new BCFile.Reader(fsin, fs.getFileStatus(path).getLen(), conf,
- CryptoServiceFactory.newInstance(siteConfig,
ClassloaderType.ACCUMULO));
+try (BCFile.Reader bcfr = new BCFile.Reader(fsin,
fs.getFileStatus(path).getLen(), conf,
Review comment:
@ctubbsii thanks for the info. I'm not that familiar with the use of var at
the moment so I will read up on it a bit and then see if I can apply it where
appropriate. You can take another look once I push another update.
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:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [accumulo] cradal commented on a change in pull request #1518: Fix #1430 Enforce table name limit for new tables
cradal commented on a change in pull request #1518: Fix #1430 Enforce table
name limit for new tables
URL: https://github.com/apache/accumulo/pull/1518#discussion_r388271674
##
File path: test/src/main/java/org/apache/accumulo/test/NamespacesIT.java
##
@@ -191,6 +192,18 @@ public void createTableInMissingNamespace() throws
Exception {
}
}
+ @Test(expected = IllegalArgumentException.class)
+ public void createNamespaceWithNamespaceLengthLimit()
+ throws AccumuloException, AccumuloSecurityException,
NamespaceExistsException {
+StringBuilder namespaceBuilder = new StringBuilder();
+for (int i = 0; i <= MAX_NAMESPACE_LEN; i++) {
+ namespaceBuilder.append('a');
+}
+String namespace = namespaceBuilder.toString();
+c.namespaceOperations().create(namespace);
+assertTrue(!c.namespaceOperations().exists(namespace));
Review comment:
The assertion line is not needed if we depend on the throw/not thrown status
of the expected exception for testing. However, that does not really test for
the existence of a table with an invalid name. So...I removed the expected
notation and put the assertion in a try-catch block in the newest commit.
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:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [accumulo] cradal commented on a change in pull request #1518: Fix #1430 Enforce table name limit for new tables
cradal commented on a change in pull request #1518: Fix #1430 Enforce table
name limit for new tables
URL: https://github.com/apache/accumulo/pull/1518#discussion_r388271716
##
File path: test/src/main/java/org/apache/accumulo/test/TableOperationsIT.java
##
@@ -189,6 +190,18 @@ public void createTable() throws TableExistsException,
AccumuloException,
accumuloClient.tableOperations().delete(tableName);
}
+ @Test(expected = IllegalArgumentException.class)
+ public void createTableWithTableNameLengthLimit()
+ throws AccumuloException, AccumuloSecurityException,
TableExistsException {
+StringBuilder tableNameBuilder = new StringBuilder();
+for (int i = 0; i <= MAX_TABLE_NAME_LEN; i++) {
+ tableNameBuilder.append('a');
+}
+String tableName = tableNameBuilder.toString();
+accumuloClient.tableOperations().create(tableName);
+assertTrue(!accumuloClient.tableOperations().exists(tableName));
Review comment:
Please see NamespacesIT comment.
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:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [accumulo] phrocker commented on a change in pull request #1545: Remove miniDFS from HalfDeadTServerIT
phrocker commented on a change in pull request #1545: Remove miniDFS from
HalfDeadTServerIT
URL: https://github.com/apache/accumulo/pull/1545#discussion_r388231472
##
File path:
test/src/main/java/org/apache/accumulo/test/functional/HalfDeadTServerIT.java
##
@@ -63,7 +66,29 @@ protected int defaultTimeoutSeconds() {
return 4 * 60;
}
- class DumpOutput extends Daemon {
+ private static final AtomicBoolean sharedLibBuilt = new AtomicBoolean(false);
+
+ @SuppressFBWarnings(value = "COMMAND_INJECTION",
+ justification = "command executed is not from user input")
+ @BeforeClass
+ public static void buildSharedLib() throws IOException, InterruptedException
{
+String root = System.getProperty("user.dir");
+String source = root + "/src/test/c/fake_disk_failure.c";
+String lib = root + "/target/fake_disk_failure.so";
Review comment:
If the "so" exists, do you even need to rec-compile? I guess you run the
risk of it being corrupt or an incorrect so, but a clean should remove this as
well.
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:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [accumulo] phrocker commented on a change in pull request #1545: Remove miniDFS from HalfDeadTServerIT
phrocker commented on a change in pull request #1545: Remove miniDFS from
HalfDeadTServerIT
URL: https://github.com/apache/accumulo/pull/1545#discussion_r388231472
##
File path:
test/src/main/java/org/apache/accumulo/test/functional/HalfDeadTServerIT.java
##
@@ -63,7 +66,29 @@ protected int defaultTimeoutSeconds() {
return 4 * 60;
}
- class DumpOutput extends Daemon {
+ private static final AtomicBoolean sharedLibBuilt = new AtomicBoolean(false);
+
+ @SuppressFBWarnings(value = "COMMAND_INJECTION",
+ justification = "command executed is not from user input")
+ @BeforeClass
+ public static void buildSharedLib() throws IOException, InterruptedException
{
+String root = System.getProperty("user.dir");
+String source = root + "/src/test/c/fake_disk_failure.c";
+String lib = root + "/target/fake_disk_failure.so";
Review comment:
If the so exists, do you even need to rec-compile? I guess you run the risk
of it being corrupt or an incorrect so, but a clean should remove this as well.
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:
us...@infra.apache.org
With regards,
Apache Git Services
