Add tool elasticsearch-node unsafe-bootstrap by andrershov · Pull Request #37696 · elastic/elasticsearch

andrershov · 2019-01-22T10:57:01Z

This tool could be used if the majority of master-eligible nodes is lost.
Docs can be found in #37812

elasticmachine · 2019-01-22T10:57:03Z

Pinging @elastic/es-distributed

andrershov · 2019-01-22T12:20:23Z

run elasticsearch-ci/2

andrershov · 2019-01-22T17:14:36Z

run elasticsearch-ci/2

DaveCTurner

I left mostly minor suggestions, but we should discuss how to deal with node ordinals >0 as I don't think we should just silently ignore them.

I will look in more depth at the docs later.

DaveCTurner · 2019-01-23T07:55:29Z

qa/vagrant/src/main/java/org/elasticsearch/packaging/test/ArchiveTestCase.java

        }
    }

+    public void test110RepairIndexCliPackaging() {


RepairIndex? I see that this was copied from the test above, which also shouldn't say Repair.

Oops, I've changed the names of both tests in this commit e2abf3f

server/src/main/java/org/elasticsearch/cluster/coordination/NodeToolCli.java

DaveCTurner · 2019-01-23T08:10:55Z

server/src/main/java/org/elasticsearch/cluster/coordination/UnsafeBootstrapMasterCommand.java

+            "\n" +
+            "    WARNING: Elasticsearch MUST be stopped before running this tool.\n" +
+            "\n" +
+            "You should run this tool only if you've lost the majority of master eligible nodes.\n" +


Suggest:

You should run this tool only if you have permanently lost half or more of the master-eligible nodes, and you cannot restore the cluster from a snapshot. This tool can result in arbitrary data loss and should be the last resort.

I'm suggesting to avoid saying "lost a majority" because you also need this tool if you have an even number of nodes and you've lost half of them, which isn't strictly a majority.

Also could you explain how it can render the cluster "completely non-functional"? I mean I see how it can break indices, but it should allow a cluster to form and any broken indices can be deleted.

0894fe4
I could not give you an example, I just wanted for this warning to sound as scary as possible.

DaveCTurner · 2019-01-23T08:12:30Z

server/src/main/java/org/elasticsearch/cluster/coordination/UnsafeBootstrapMasterCommand.java

+            ClusterService.USER_DEFINED_META_DATA.getConcreteSetting("cluster.metadata.unsafe-bootstrap");
+
+    UnsafeBootstrapMasterCommand() {
+        super("Unsafely bootstraps the master node if the majority of master eligible nodes is lost");


Again, not strictly a majority. Maybe we should replace "bootstraps" with "forces the successful election of"?

How do you find this message b6ada54?

server/src/main/java/org/elasticsearch/cluster/coordination/UnsafeBootstrapMasterCommand.java

DaveCTurner · 2019-01-23T08:35:09Z

server/src/main/java/org/elasticsearch/cluster/coordination/UnsafeBootstrapMasterCommand.java

+        try {
+            terminal.println(Terminal.Verbosity.VERBOSE, "Writing new global metadata to disk");
+            long newGeneration = MetaData.FORMAT.write(newMetaData, dataPaths);
+            Manifest newManifest = new Manifest(manifest.getCurrentTerm(), manifest.getClusterStateVersion(), newGeneration,


I think we should increment the term, and log that we've done so.

I've implemented increment in this commit 0e50913. For now, I'm logging new currentTerm to the terminal, will get back to it - once I figure out how our command line tools work with elasticsearch logs.

DaveCTurner · 2019-01-23T08:36:40Z

server/src/main/java/org/elasticsearch/cluster/coordination/UnsafeBootstrapMasterCommand.java

+    @Override
+    protected void execute(Terminal terminal, OptionSet options, Environment env) throws Exception {
+        // We don't use classical "private static final Logger", because log4j2 initialization happens
+        // after UnsafeBootstrapMasterCommand instance creation.


Hmm, we make a private static final Logger logger in RemoveCorruptedShardDataCommand, does that not work?

Also, does this logger have the same configuration as loggers in Elasticsearch proper, i.e., it writes to the Elasticsearch log by default? If so, I think we should log more information about this tool having been run in that log.

Ok, I have figured out how the logging works:

Most of the commands, for example, LoggingAwareCommand are passing CommandLoggingConfigurator::configureLoggingWithoutConfig callback to Command constructor as beforeMain argument. This is the function that will be executed when Command.main is called.

From CommandLoggingConfigurator::configureLoggingWithoutConfig javadoc Configure logging without reading a log4j2.properties file, effectively configuring the status logger and all loggers to the console. Basically it means that logs won't be appended to elasticsearch log files.

If you run ShardToolCli indeed you get ERROR StatusLogger No Log4j 2 configuration file found. Using default configuration. The reason for this is because logger initialization happens when log4j2 classes are loaded. Java Runtime loads classes when it first encounters this class, in case of ShardToolCli it will load RemoveCorruptedShardDataCommand and transitively Logger class, because there is static reference to Logger class in RemoveCorruptedShardDataCommand. Since pre-main is not yet executed, logging is not configured.

It turned out that NodeToolCli still suffers from the same warning, because there is ClusterService (which in turn staticly references Logger) static reference in UnsafeBootstrapMasterCommand. The reason why I've not noticied it earlier, because I've added this setting last moment, after I've checked how binaries are working.

For now I've made the following fix e23f59a

I suggest to revisit how logging works for commands in general in the follow-up PR. It seems that beforeMain could be completely removed and replaced by logging initialization in the command constructor. But this change touches a lot of file and I prefer separate PR.

DaveCTurner · 2019-01-23T08:38:04Z

server/src/test/java/org/elasticsearch/cluster/coordination/UnsafeBootstrapMasterIT.java

+        internalCluster().startNode(
+                Settings.builder()
+                        .put(ElectMasterService.DISCOVERY_ZEN_MINIMUM_MASTER_NODES_SETTING.getKey(), Integer.MAX_VALUE)
+                        .put(DiscoverySettings.INITIAL_STATE_TIMEOUT_SETTING.getKey(), "2s") //to ensure quick node startup


Why not zero?

server/src/test/java/org/elasticsearch/cluster/coordination/UnsafeBootstrapMasterIT.java

DaveCTurner · 2019-01-23T08:42:20Z

server/src/test/java/org/elasticsearch/cluster/coordination/UnsafeBootstrapMasterIT.java

+            Settings lastNodeSettings = allNodesSettings.get(allNodesSettings.size() - 1);
+            List<Settings> newSettings = new ArrayList<>();
+            newSettings.addAll(otherNodesSettings);
+            newSettings.add(Settings.builder().put(lastNodeSettings)


Could we bootstrap a random node rather than it always being the last of them?

Co-Authored-By: andrershov <andrershov@gmail.com>

andrershov · 2019-01-23T13:39:23Z

@DaveCTurner thanks for your review! I've addressed all the comments except nodeOrdinal, I think it's ready for the 2nd review pass.

DaveCTurner

Ok, looks good apart from the removal of the docs that we discussed.

server/src/test/java/org/elasticsearch/cluster/coordination/UnsafeBootstrapMasterIT.java

server/src/main/java/org/elasticsearch/cluster/coordination/UnsafeBootstrapMasterCommand.java

DaveCTurner · 2019-01-24T11:40:39Z

docs/reference/commands/node-tool.asciidoc

@@ -0,0 +1,119 @@
+[[node-tool]]
+== elasticsearch-node


We decided to move the docs changes to a separate PR - #37812, so this should go.

DaveCTurner

LGTM

andrershov · 2019-01-24T12:56:15Z

run elasticsearch-ci/1

andrershov · 2019-01-24T15:47:27Z

run elasticsearch-ci/1

DaveCTurner

LGTM again

@DaveCTurner

This commit, mostly authored by @DaveCTurner, adds documentation for elasticsearch-node tool #37696.

@DaveCTurner

This commit, mostly authored by @DaveCTurner, adds documentation for elasticsearch-node tool #37696. (cherry picked from commit 09425d5)

@DaveCTurner

This commit, mostly authored by @DaveCTurner, adds documentation for elasticsearch-node tool #37696. (cherry picked from commit 09425d5)

Andrey Ershov added 3 commits January 22, 2019 11:09

elasticsearch-node unsafe-bootstrap tool

bf7de89

Add unsafe bootstrap persistent setting

38fc215

Add sample tool output to docs

35de73f

andrershov added >enhancement :Distributed/Cluster Coordination Cluster formation and cluster state publication, including cluster membership and fault detection. labels Jan 22, 2019

andrershov requested a review from ywelsch January 22, 2019 10:57

Fix technical issues with docs

9aff0d5

ywelsch requested a review from DaveCTurner January 22, 2019 12:25

ywelsch mentioned this pull request Jan 22, 2019

A new cluster coordination layer #32006

Closed

61 tasks

DaveCTurner requested changes Jan 23, 2019

View reviewed changes

Andrey Ershov and others added 9 commits January 23, 2019 12:19

Rename tests

e2abf3f

NodeToolCli description change

f0a81ce

Co-Authored-By: andrershov <andrershov@gmail.com>

Warning message update

0894fe4

UnsafeBootstrapMasterCommand description change

b6ada54

Fix help message in ArchiveTestCase

09ef282

Increment currentTerm

0e50913

Change timeout to 0

6552d54

Bootstrap random node from the last batch

f3d78c4

Move logging initialization to NodeToolCli

e23f59a

andrershov mentioned this pull request Jan 24, 2019

Add elasticsearch-node tool docs #37812

Merged

Merge branch 'master' into zen2_node_tool

cd6dfa0

DaveCTurner reviewed Jan 24, 2019

View reviewed changes

Remove docs

4036edb

DaveCTurner approved these changes Jan 24, 2019

View reviewed changes

Ask for confirmation after the checks, output term/version

f1a3fa9

andrershov removed the request for review from ywelsch January 24, 2019 15:11

DaveCTurner approved these changes Jan 24, 2019

View reviewed changes

andrershov merged commit 4974684 into elastic:master Jan 24, 2019

tomcallahan added the v7.0.0 label Jan 25, 2019

colings86 added v7.0.0-beta1 and removed v7.0.0 labels Feb 7, 2019

andrershov added a commit that referenced this pull request Mar 12, 2019

Add elasticsearch-node tool docs (#37812)

09425d5

This commit, mostly authored by @DaveCTurner, adds documentation for elasticsearch-node tool #37696.

andrershov pushed a commit that referenced this pull request Mar 12, 2019

Add elasticsearch-node tool docs (#37812)

b3a7262

This commit, mostly authored by @DaveCTurner, adds documentation for elasticsearch-node tool #37696. (cherry picked from commit 09425d5)

andrershov pushed a commit that referenced this pull request Mar 12, 2019

Add elasticsearch-node tool docs (#37812)

aa967ed

This commit, mostly authored by @DaveCTurner, adds documentation for elasticsearch-node tool #37696. (cherry picked from commit 09425d5)

Conversation

andrershov commented Jan 22, 2019 • edited by ywelsch Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

elasticmachine commented Jan 22, 2019

Uh oh!

andrershov commented Jan 22, 2019

Uh oh!

andrershov commented Jan 22, 2019

Uh oh!

DaveCTurner left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

andrershov commented Jan 23, 2019

Uh oh!

DaveCTurner left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

DaveCTurner left a comment

Choose a reason for hiding this comment

Uh oh!

andrershov commented Jan 24, 2019

Uh oh!

andrershov commented Jan 24, 2019

Uh oh!

DaveCTurner left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

andrershov commented Jan 22, 2019 •

edited by ywelsch

Loading