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 2022/08/04 17:24:16 UTC

[GitHub] [accumulo] milleruntime opened a new pull request, #2850: Make Mini call initialize code directly

milleruntime opened a new pull request, #2850:
URL: https://github.com/apache/accumulo/pull/2850

   * Refactor MiniAccumuloClusterImpl to call Initialize code directly instead of running in a separate process


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@accumulo.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [accumulo] milleruntime commented on pull request #2850: Make Mini call initialize code directly

Posted by GitBox <gi...@apache.org>.
milleruntime commented on PR #2850:
URL: https://github.com/apache/accumulo/pull/2850#issuecomment-1205622570

   There are a several ITs failing:
   <pre>
   [ERROR]   AccumuloClientIT.testClose:185 expected: CLIEN> but was: SERVER
   [ERROR]   CleanUpIT.run execution timed out after 30000 ms
   </pre>
    
   These are a bunch failing for the same reason during setupCluster(): 
   <pre>
   java.lang.IllegalStateException: Could not set manager goal state, process returned 1. Check the logs ...
   Main_1684807532.out:
   2022-08-04T14:24:37,396 [fs.VolumeManager] ERROR: multiple potential instances in file:/home/mpmill4/workspace/accumulo/test/target/mini-tests/org.apache.accumulo.test.functional.CreateInitialSplitsIT_testCreateInitialSplits/accumulo/instance_id
   2022-08-04T14:24:37,397 [start.Main] ERROR: Thread 'org.apache.accumulo.manager.state.SetGoalState' died.
   java.lang.RuntimeException: Accumulo found multiple possible instance ids in file:/home/mpmill4/workspace/accumulo/test/target/mini-tests/org.apache.accumulo.test.functional.CreateInitialSplitsIT_testCreateInitialSplits/accumulo/instance_id
           at org.apache.accumulo.server.fs.VolumeManager.getInstanceIDFromHdfs(VolumeManager.java:222) ~[classes/:?]
           at org.apache.accumulo.server.ServerInfo.<init>(ServerInfo.java:102) ~[classes/:?]
           at org.apache.accumulo.server.ServerContext.<init>(ServerContext.java:110) ~[classes/:?]
           at org.apache.accumulo.manager.state.SetGoalState.main(SetGoalState.java:48) ~[classes/:?]
           at jdk.internal.reflect.DirectMethodHandleAccessor.invoke(DirectMethodHandleAccessor.java:104) ~[?:?]
           at java.lang.reflect.Method.invoke(Method.java:577) ~[?:?]
           at org.apache.accumulo.start.Main.lambda$execMainClass$1(Main.java:163) ~[classes/:?]
           at java.lang.Thread.run(Thread.java:833) ~[?:?]
   
   </pre>
   CompactionIT
   CreateInitialSplitsIT
   CleanTmpIT
   CleanWalIT
   BulkNewIT
   BulkImportVolumeIT
   BulkImportSequentialRowsIT


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@accumulo.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [accumulo] EdColeman commented on a diff in pull request #2850: Make Mini call initialize code directly

Posted by GitBox <gi...@apache.org>.
EdColeman commented on code in PR #2850:
URL: https://github.com/apache/accumulo/pull/2850#discussion_r938831507


##########
minicluster/src/main/java/org/apache/accumulo/miniclusterImpl/MiniAccumuloClusterImpl.java:
##########
@@ -581,13 +581,16 @@ public synchronized void start() throws IOException, InterruptedException {
         }
 
         log.warn("Initializing ZooKeeper");
-        try (var vm = VolumeManagerImpl.get(siteConfig, config.getHadoopConfiguration())) {
+        var hadoopConfig = new Configuration(false);
+        File csFile = new File(config.getConfDir(), "core-site.xml");
+        hadoopConfig.addResource(csFile.toURI().toURL());
+
+        try (var vm = VolumeManagerImpl.get(siteConfig, hadoopConfig)) {
           log.info("Calling init with {}", args);
           Initialize init = new Initialize();
           Initialize.Opts opts = new Initialize.Opts();
           opts.parseArgs("accumulo init", args.toArray(new String[0]));
-          InitialConfiguration initConfig =
-              new InitialConfiguration(config.getHadoopConfiguration(), siteConfig);
+          InitialConfiguration initConfig = new InitialConfiguration(hadoopConfig, siteConfig);

Review Comment:
   To be clear - I was asking if it would help in the future - so that if there are other problems, it is always available to help see what the configuration is, rather than guessing.  It could be being logged elsewhere, I do not know.  Figured you'd be running the tests and you would be in a position to evaluate if something would help, if it is not available now.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@accumulo.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [accumulo] milleruntime commented on a diff in pull request #2850: Make Mini call initialize code directly

Posted by GitBox <gi...@apache.org>.
milleruntime commented on code in PR #2850:
URL: https://github.com/apache/accumulo/pull/2850#discussion_r938826284


##########
minicluster/src/main/java/org/apache/accumulo/miniclusterImpl/MiniAccumuloClusterImpl.java:
##########
@@ -581,13 +581,16 @@ public synchronized void start() throws IOException, InterruptedException {
         }
 
         log.warn("Initializing ZooKeeper");
-        try (var vm = VolumeManagerImpl.get(siteConfig, config.getHadoopConfiguration())) {
+        var hadoopConfig = new Configuration(false);
+        File csFile = new File(config.getConfDir(), "core-site.xml");
+        hadoopConfig.addResource(csFile.toURI().toURL());
+
+        try (var vm = VolumeManagerImpl.get(siteConfig, hadoopConfig)) {
           log.info("Calling init with {}", args);
           Initialize init = new Initialize();
           Initialize.Opts opts = new Initialize.Opts();
           opts.parseArgs("accumulo init", args.toArray(new String[0]));
-          InitialConfiguration initConfig =
-              new InitialConfiguration(config.getHadoopConfiguration(), siteConfig);
+          InitialConfiguration initConfig = new InitialConfiguration(hadoopConfig, siteConfig);

Review Comment:
   I think I figured it out. The ones that override `configureMiniCluster()` will write out a core-site.xml. The other ones don't so I just need to check if the file exists.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@accumulo.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [accumulo] milleruntime commented on pull request #2850: Make Mini call initialize code directly

Posted by GitBox <gi...@apache.org>.
milleruntime commented on PR #2850:
URL: https://github.com/apache/accumulo/pull/2850#issuecomment-1206510435

   > This seems to be causing more problems than what it fixes. 
   
   I think so. Closing as a failed experiment.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@accumulo.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [accumulo] ctubbsii commented on pull request #2850: Make Mini call initialize code directly

Posted by GitBox <gi...@apache.org>.
ctubbsii commented on PR #2850:
URL: https://github.com/apache/accumulo/pull/2850#issuecomment-1206504831

   This seems to be causing more problems than what it fixes. Even if we work through those issues with the broken ITs, I'm not so sure this will yield a good end result. See my comments on https://github.com/apache/accumulo/pull/2846#issuecomment-1206499320 regarding the benefits of initializing mini as a separate server process.
   
   In general, it seems very strange to make the IT itself become the process that initializes Accumulo, when Mini it still launching separate processes for all the other server components of the cluster.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@accumulo.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [accumulo] milleruntime closed pull request #2850: Make Mini call initialize code directly

Posted by GitBox <gi...@apache.org>.
milleruntime closed pull request #2850: Make Mini call initialize code directly
URL: https://github.com/apache/accumulo/pull/2850


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@accumulo.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [accumulo] milleruntime commented on pull request #2850: Make Mini call initialize code directly

Posted by GitBox <gi...@apache.org>.
milleruntime commented on PR #2850:
URL: https://github.com/apache/accumulo/pull/2850#issuecomment-1206331453

   The ITs that are failing all use
   <pre>
   conf.set("fs.file.impl", RawLocalFileSystem.class.getName());
   </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.

To unsubscribe, e-mail: notifications-unsubscribe@accumulo.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [accumulo] EdColeman commented on a diff in pull request #2850: Make Mini call initialize code directly

Posted by GitBox <gi...@apache.org>.
EdColeman commented on code in PR #2850:
URL: https://github.com/apache/accumulo/pull/2850#discussion_r938798000


##########
minicluster/src/main/java/org/apache/accumulo/miniclusterImpl/MiniAccumuloClusterImpl.java:
##########
@@ -581,13 +581,16 @@ public synchronized void start() throws IOException, InterruptedException {
         }
 
         log.warn("Initializing ZooKeeper");
-        try (var vm = VolumeManagerImpl.get(siteConfig, config.getHadoopConfiguration())) {
+        var hadoopConfig = new Configuration(false);
+        File csFile = new File(config.getConfDir(), "core-site.xml");
+        hadoopConfig.addResource(csFile.toURI().toURL());
+
+        try (var vm = VolumeManagerImpl.get(siteConfig, hadoopConfig)) {
           log.info("Calling init with {}", args);
           Initialize init = new Initialize();
           Initialize.Opts opts = new Initialize.Opts();
           opts.parseArgs("accumulo init", args.toArray(new String[0]));
-          InitialConfiguration initConfig =
-              new InitialConfiguration(config.getHadoopConfiguration(), siteConfig);
+          InitialConfiguration initConfig = new InitialConfiguration(hadoopConfig, siteConfig);

Review Comment:
   Any benefit to logging the config (at debug level?) here to help show what it is running with (or not running if it fails?)



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@accumulo.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org