Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Incomplete tactical implementation for -XX:+PrintFlagsFinal #9567

Merged
merged 1 commit into from
May 27, 2020

Conversation

mstoodle
Copy link
Contributor

@mstoodle mstoodle commented May 14, 2020

An incomplete implementation of VM options that produces the
-XX:+PrintFlagsFinal output for two specific options required by
the elasticsearch project. This code is expected to be temporary
until a more comprehensive solution can be architected. For this
reason, this change is very straight-forward and localized so
it can be easily removed.

The two options whose values will be printed are for
-XX:MaxHeapSize= and -XX:MaxDirectMemorySize= .

With this change, elasticsearch no longer NPEs on startup and,
in my local testing, all their tests pass on OpenJ9.

I tried to match, as closely as possible, the output Hotspot
generates when one uses -XX:+PrintFlagsFinal, which includes the
names used for types (e.g. size_t or uint64_t), the option names,
the option category (e.g. {product} for these two options), the
way the option is set (e.g. {ergonomic} or {command line} for
these two options), and the field widths produced by a JDK11
Hotspot build.

I added 3 command-line tests :

  1. that two options (MaxHeapSize, MaxDirectMemorySize) are present
    when -XX:+PrintFlagsFinal option is specified.

  2. that -XX:+PrintFlagsFinal overrides earlier -XX:-PrintFlagsFinal.

  3. that -XX:-PrintFlagsFinal overrides earlier -XX:+PrintFlagsFinal.

Fixes: #7764

Doc issue eclipse-openj9/openj9-docs#571

Signed-off-by: Mark Stoodley mstoodle@ca.ibm.com

runtime/vm/options.c Outdated Show resolved Hide resolved
@mstoodle mstoodle force-pushed the PrintFlagsFinal branch 2 times, most recently from f17ba11 to 43fdb2d Compare May 15, 2020 04:15
@mstoodle mstoodle changed the title WIP: Simple incomplete implementation for PrintFlagsFinal Simple incomplete implementation for PrintFlagsFinal May 15, 2020
@mstoodle mstoodle requested a review from pshipton May 15, 2020 15:04
runtime/vm/options.c Outdated Show resolved Hide resolved
@@ -6349,6 +6350,15 @@ protectedInitializeJavaVM(J9PortLibrary* portLibrary, void * userData)
Xj9BreakPoint("jvminit");
#endif

{
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should indent via tabs to match the surrounding code.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

bah, i thought I had caught all those. Thanks!

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The opening brace should be indented one tab less than the block it begins (unlike JIT code).

@pshipton
Copy link
Member

We should have a test added for -XX:+PrintFlagsFinal to ensure it prints something reasonable and doesn't assert. It can also incorporate testing to ensure the last -XX:[+/-]PrintFlagsFinal option wins.

@mstoodle
Copy link
Contributor Author

Thanks for the review, @pshipton . That's a good point about tests. I'll put something together.

Copy link
Member

@DanHeidinga DanHeidinga left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@mstoodle Thanks for pulling together this PR to help get Elasticsearch running on OpenJ9.

While I support the goal, I'm concerned about the approach taken here. Our option parsing code is already messy for historical reasons and from being spread across the various components (and projects - including split between openj9 and omr). This seems to add another inconsistent way to handle argument parsing without a story on how to fix this long term.

Without a better understanding of how to address the larger issue of option parsing in the project, this doesn't feel like a step towards a solution.

For the goal of unblocking Elasticsearch, -XX:+PrintFlagsFinal could be emulated using the existing option parsing code with an extra loop through the vm->vmArgsArray looking for the -Xmx / -XX:MaxHeapSize= options and querying the GC for the actual setting. The options code can already differentiate between commandline and envvars.

@@ -334,7 +336,7 @@ enum INIT_STAGE {

#define VMOPT_XSOFTREFTHRESHOLD "-XSoftRefThreshold"
#define VMOPT_XAGGRESSIVE "-Xaggressive"
#define VMOPT_XXMAXDIRECTMEMORYSIZEEQUALS "-XX:MaxDirectMemorySize="
#define VMOPT_XXMAXDIRECTMEMORYSIZEEQUALS (VMOPT_XXMAXDIRECTMEMORYSIZE "=")
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a neat approach but different than we do with other options. I'd prefer not to introduce this difference for one option without a compelling reason. Same rationale for -XX:MaxHeapSize= below

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'll put it back, then.

@@ -542,6 +544,12 @@ enum INIT_STAGE {
#define VMOPT_XXSHOW_EXTENDED_NPE_MESSAGE "-XX:+ShowCodeDetailsInExceptionMessages"
#define VMOPT_XXNOSHOW_EXTENDED_NPE_MESSAGE "-XX:-ShowCodeDetailsInExceptionMessages"

/* Print flag values */
#define VMOPT_XXPRINTFLAGSINITIALENABLE "-XX:+PrintFlagsInitial"
#define VMOPT_XXPRINTFLAGSINITIALDISABLE "-XX:-PrintFlagsInitial"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't see these being used anywhere. Did I miss something?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

no, they were a "freebie" ...I'll remove them :)

@pshipton
Copy link
Member

For the goal of unblocking Elasticsearch, -XX:+PrintFlagsFinal could be emulated using the existing option parsing code with an extra loop through the vm->vmArgsArray looking for the -Xmx / -XX:MaxHeapSize= options and querying the GC for the actual setting.

Isn't this what this PR does?

@DanHeidinga
Copy link
Member

Isn't this what this PR does?

Yes, but wrapped in a new option setting framework. There's ~20 lines of code that should be needed to handle this.

Unless we plan to adopt this approach for all options, and to gradually migrate the rest of the options into this framework, it would be hard to guide someone to know when to use this vs the existing option processing code.

@pshipton
Copy link
Member

Unless we plan to adopt this approach for all options, and to gradually migrate the rest of the options into this framework

I believe that is the intention, to gradually add more options into this framework as time permits, in order to include them in the PrintFlagsFinal output.

it would be hard to guide someone to know when to use this vs the existing option processing
code.

This framework doesn't affect the existing option processing code, it handles the details necessary to get PrintFlagsFinal output in a generic way so additional options can be added in the future.

@mstoodle
Copy link
Contributor Author

@DanHeidinga I was assuming other uses of PrintFlagsFinal may come up, so I built something a little bit more elaborate than the "20 lines" of code it would require to one-off it for elasticsearch's current use case. We have had independent requests for PrintFlagsFinal to be implemented in OpenJ9, and it seems likely that once we have something in place, we may get requests to add specific options to the output.

I was striving to balance enabling a simple way to extend the framework with additional options without building something baroque (or capable of unifying our existing multiple options processing frameworks). I think the guidance on which options framework to use boils down to whether you want to update PrintFlagsFinal or something else.

In the end, this code resides in a component I am not well versed in and am unlikely to be strongly impacted by the larger impacts of the code I'm contributing (though obviously I'll take full responsibility for this facility), so I will happily adjust the PR according to @DanHeidinga 's and @pshipton 's opinions. It doesn't sound like we've quite settled on the approach yet.

I would very much like to have some solution well in place for the July release.

@DanHeidinga
Copy link
Member

@mstoodle I've added this to the 0.21 milestone to ensure that's addressed for the next quarterly update of OpenJ9

Comment on lines 32 to 35
#define VMOPT_XXMAXHEAPSIZE XXCOLON VMOPT_MAXHEAPSIZE

#define VMOPT_MAXDIRECTMEMORYSIZE "MaxDirectMemorySize"
#define VMOPT_XXMAXDIRECTMEMORYSIZE XXCOLON VMOPT_MAXDIRECTMEMORYSIZE
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Either parentheses are missing in the VMOPT_XX definitions or (as it seems @DanHeidinga would prefer) the -XX: prefix should just be folded into the string literals.

@@ -80,6 +80,7 @@
#include "bcnames.h"
#include "jimagereader.h"
#include "vendor_version.h"
#include "options.h"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This include is redundant (see jvminit.h).

#define TYPE_UINT64T 8
#define TYPE_UINTX 9

static char *option_type_string[] = {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The type should be const char * const[].

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

good point, thanks

"uint64_t",
"uintx"
};
#define NUM_TYPE_STRINGS (sizeof(option_type_string) / sizeof(char *))
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggest: (sizeof(option_type_string) / sizeof(option_type_string[0])) so it can't be out of sync with the element type. Likewise for NUM_CATEGORY_STRINGS and NUM_HOWSET_STRINGS, etc. below.

Comment on lines 152 to 160
#define SET_OPTION0(option, cmdLineOption, vtype, setValue, defaultSource, OTHER_CHECKS) \
assert(option < NUM_OPTIONS); \
o = options + option; \
o->value.vtype = (setValue); \
if ((findArgInVMArgs( PORTLIB, vm->vmArgsArray, STARTSWITH_MATCH, cmdLineOption, NULL, 0) >= 0) OTHER_CHECKS) { \
o->howset = SET_COMMANDLINE; \
} else { \
o->howset = (defaultSource); \
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should be surrounded by do { ... } while (0) to avoid syntactic surprises.
Uses of macro arguments should be parenthesized.
o should be a local variable in that do loop (perhaps with a better name).

Comment on lines 216 to 218
UDATA o_idx=0;
for (o_idx = 0;o_idx < NUM_OPTIONS;o_idx++) {
J9OptionDesc *o = options + o_idx;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Formatting: space around = and after ;.

runtime/vm/options.c Outdated Show resolved Hide resolved
* SPDX-License-Identifier: EPL-2.0 OR Apache-2.0 OR GPL-2.0 WITH Classpath-exception-2.0 OR LicenseRef-GPL-2.0 WITH Assembly-exception
*******************************************************************************/

OPTION(VMOPT_MAXHEAPSIZE, TYPE_SIZET, CAT_PRODUCT)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This would benefit from a comment explaining how these are used.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fair point, will add some if this implementation is what's chosen to move forward with.

Thanks for the review @keithc-ca !

@pshipton
Copy link
Member

pshipton commented May 19, 2020

@mstoodle I discussed with @DanHeidinga and this what we came up with for further discussion. This solution suggests to always build the options table and populate the data, which is a runtime cost, however doing this supports using jcmd and MXBeans to get options, which is another missing OpenJ9 feature. Note the MXBean also allows some option values to be modified at runtime. The following could be extended to handle this later.

In order to get the two options delivered for 0.21, it's acceptable to put a simplified version of the code in initializeFlags() into jvminit.c to update the options table.

  • Create a table of options to include in PrintFlags, basically like J9OptionDesc. This table could be per component since each component has separate options parsing, although it might be easier to just have one and put it into the J9JavaVM. If there are multiple tables, there needs to be a way to find and iterate through them all. Create an enum or define for each option's table index, to be used below. The table should have a slot for the default value as well as the current value, to support PrintFlagsInitial. The default value could be hard coded or filled in at runtime. Some values are computed, so need to be filled in at runtime. Not sure of the value/priority for implementing PrintFlagsInitial.

  • Add fields to the J9CmdLineOption struct to help manage the options table. Interestingly, this struct is part of OMR, although I don't see any uses of it in OMR by searching the code. The new fields store the index of the option in the option table and the current value. Some value (like -1 or 0) indicates the option isn't supported by the option table, so can be ignored. J9CmdLineOption already gives information on how the option is set (environment or command line). See the next comment Incomplete tactical implementation for -XX:+PrintFlagsFinal #9567 (comment).

  • The code which parses each option updates the J9CmdLineOption, which it should have already determined, with the table index and option value associated option table entry, and gets information about how the option is set from the J9CmdLineOption. This can avoid yet another search through the options, but it's up to the parsing code / component to manage it.

  • After all options have been parsed, iterate once through the J9CmdLineOption's and update the option table with the current value and how set.

  • PrintFlagsInitial and PrintFlagsFinal iterate the option table and print the values.

@pshipton
Copy link
Member

Dan and I talked about adding fields to J9CmdLineOption, but now that I've saved the last comment, not sure this provides much value. The option parsing code could just directly update the current value in the option table. By providing the J9CmdLineOption index to a helper function used to update the table, the origin of the option can be derived.

@mstoodle
Copy link
Contributor Author

mstoodle commented May 20, 2020

Thanks for taking the time to consider the grander design as part of this PR.

In Hotspot, I believe (though obviously I don't know for sure) that the only difference between PrintFlagsInitial and PrintFlagsFinal is when the printing code is called. That's why I did not add a "default" field into the current J9OptionDesc structure.

This approach also suggests that 1) the array of options is statically allocated and filled in (or at least filled in prior to any command line option processing), and 2) the default values are initialized into the table as the starting value. In particular, the printing code could not rely at all on the command line option processing code. Is there independent value inside the JVM in knowing the default value of an option that has been changed on the command line?

I generally like what you've described and would like to try to implement as much of the facility as possible for 0.21, but propose to deliver the "simplified" solution you describe first (via this PR) and extend it via other PRs. It sounds like you would accept this approach, but I would like to understand your position on the above before finalizing the state of this PR.

@keithc-ca
Copy link
Contributor

We can't rely on the compiler/linker to statically initialize that table because clients may create a series of JVMs via JNI_CreateJavaVM() and DestroyJavaVM() (e.g. like eclipse does).

@mstoodle
Copy link
Contributor Author

Not sure why the table of options (most of which doesn't change) couldn't be statically allocated (though it would mean that the default values for options would need to be stored independently of the current value, since obviously the current values could change).

@pshipton
Copy link
Member

It's fine with me to ignore the default values until we find some reason that PrintFlagsInitial needs to be implemented.

@pshipton
Copy link
Member

In particular, the printing code could not rely at all on the command line option processing code.

I'm not clear on what is meant by this point. If we were to support default values, these could be filled in at the same time as the current values, either while or after processing the command line arguments. PrintFlagsInitial / PrintFlagsFinal would just print the table. I didn't have any expectation that default values would all be added to the table in advance of command line processing, although I suppose that is an alternate approach. As I already wrote, until somebody comes up with a reason why PrintFlagsInitial is needed, I'm happy to ignore adding any support for default values.

@mstoodle
Copy link
Contributor Author

mstoodle commented May 20, 2020

I wasn't as clear as I could have been in my earlier message. By "this approach also suggests" I had meant the assumed Hotspot approach to producing the output that I had just outlined, not the approach you described earlier. If the initial printout is done before any command-line processing, then you can't rely on command-line processing to fill in the default values.

Maybe I've misinterpreted what the suggested solution is, but I guess I'm also worried that relying on the option processing code to initialize even default values means that the processing code for every option (that we want to output in either PrintFlags* option) must always run. We wouldn't be able to have the lack of/presence of one option, for example, selectively enable the processing of other options (because then we wouldn't always get their default values). Maybe that doesn't happen today, I haven't looked. I sort of sidestepped the issue in my PR by having initializeFlags() fill in the values after command line processing as well as determining whether the value had been set on the command line.

Default option values can't be ignored, I'm afraid, because even PrintFinalFlags needs to print the default values (as well as identifying them as default or ergonomic values) if they aren't specified on the command line. However, I'll try to put something together in line with what's been suggested and we can iterate from there. I am off for most of the rest of today, but will try to put something together tomorrow.

@pshipton
Copy link
Member

because even PrintFinalFlags needs to print the default values

What I meant is that we don't need to populate a defaultValue field, we only need the currentValue field, no matter how the option value is determined.

@mstoodle
Copy link
Contributor Author

Just for awareness: I believe I've fixed most of the comments for this initial PR. But one outstanding item is to write some tests, so this isn't ready to go yet. For that reason, I'm going to mark this PR as WIP for the moment, but I'm hoping to remove that in the next day or two.

Copy link
Member

@DanHeidinga DanHeidinga left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @mstoodle. Apart from some minor changes, mostly formatting, , this looks good.

Comment on lines 6356 to 6357
UDATA maxHeapSize, maxDirectMemorySize;
char *howset;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

New VM code typically initializes all variables and tries to only have 1 per line.

Suggested change
UDATA maxHeapSize, maxDirectMemorySize;
char *howset;
UDATA maxHeapSize = 0;
UDATA maxDirectMemorySize = vm->directByteBufferMemoryMax;
char *howset = NULL;

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The types of maxHeapSize and maxDirectMemorySize should be uint64_t to match the comment below.
howset should have type const char *.

Copy link
Contributor Author

@mstoodle mstoodle May 25, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'll fix the decls.

The types of maxHeapSize and maxDirectMemorySize should be uint64_t to match the comment below.

I'm confused by this comment, because j9gc_get_maximum_heap_size() returns UDATA and vm->directByteBufferMemoryMax is declared as UDATA.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

See line 6364 below. Rather than both uint64_t, we should have size_t maxHeapSize;,

Copy link
Contributor Author

@mstoodle mstoodle May 25, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there a reason I should use size_t explicitly rather than just UDATA? For most purposes in this PR, size_t is just a string that's printed in the output because that's how the Hotspot output refers to a machine word sized unsigned integer. The values in OpenJ9 are explicitly declared as UDATA so I thought to continue to refer to them as such.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If you're trying to match the output of an implementation that says size_t and uint64_t why would you use other types for those locals? The compiler will happily widen the values where necessary. Once you've settled on types for the locals, the choice of formatting must match.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was thinking it felt a strange precedent to have Hotspot's implementation choices determine what types we should use for these values in OpenJ9 but, from a consumer standpoint, having all implementations produce as similar as possible output seems desirable. Anywhere it will be different, we need to ask ourselves if it really needs to be different, and I don't think it needs to be different in these specific cases. We probably won't get the reverse consideration, but such is life I guess.

Given all that, I'll put them back to size_t and uint64_t.

runtime/vm/jvminit.c Outdated Show resolved Hide resolved
runtime/vm/jvminit.c Outdated Show resolved Hide resolved
Comment on lines 6356 to 6357
UDATA maxHeapSize, maxDirectMemorySize;
char *howset;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The types of maxHeapSize and maxDirectMemorySize should be uint64_t to match the comment below.
howset should have type const char *.

runtime/vm/jvminit.c Show resolved Hide resolved
@@ -6349,6 +6350,15 @@ protectedInitializeJavaVM(J9PortLibrary* portLibrary, void * userData)
Xj9BreakPoint("jvminit");
#endif

{
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The opening brace should be indented one tab less than the block it begins (unlike JIT code).

@mstoodle
Copy link
Contributor Author

For my last update, I just added a new commit to address comments so they wouldn't be orphaned. Once everyone is happy, I'll squash to a single commit and rebase.

@mstoodle
Copy link
Contributor Author

This one is ready for re-review now, @DanHeidinga, at your leisure. Once you're good with it, I'll squash it down to one commit and rebase before merging.

@DanHeidinga
Copy link
Member

The only remaining thing I see is the spaces vs tabs issue mentioned in #9567 (comment) for the opening {.

Otherwise, this looks good to me

@mstoodle
Copy link
Contributor Author

There should be a "chagrin" emoji :( .

@mstoodle
Copy link
Contributor Author

I squashed the review commits and rebased (and changed those spaces to a tab).

@keithc-ca
Copy link
Contributor

@mstoodle Do you intend to improve the commit message and the subject of this PR (I don't think we want this to go down in history as a 'hack implementation')?

@mstoodle
Copy link
Contributor Author

@keith-ca, I'll soften it to "tactical".

@mstoodle mstoodle changed the title Incomplete hack implementation for -XX:+PrintFlagsFinal Incomplete tactical implementation for -XX:+PrintFlagsFinal May 26, 2020
@keithc-ca
Copy link
Contributor

@keith-ca, I'll soften it to "tactical".

The commit message still says:

Incomplete hack implementation for -XX:+PrintFlagsFinal

An incomplete implementation of VM options that produces the
-XX:+PrintFlagsFinal output for two specific options required by
the elasticsearch project. This code is expected to be temporary
until a more comprehensive solution can be architected. For this
reason, this change is very straight-forward and localized so
it can be easily removed.

The two options whose values will be printed are for
`-XX:MaxHeapSize=` and `-XX:MaxDirectMemorySize=` .

With this change, elasticsearch no longer NPEs on startup and,
in my local testing, all their tests pass on OpenJ9.

I tried to match, as closely as possible, the output Hotspot
generates when one uses -XX:+PrintFlagsFinal, which includes the
names used for types (e.g. size_t or uint64_t), the option names,
the option category (e.g. {product} for these two options), the
way the option is set (e.g. {ergonomic} or {command line} for
these two options), and the field widths produced by a JDK11
Hotspot build.

I added 3 command-line tests :

1) that two options (MaxHeapSize, MaxDirectMemorySize) are present
when -XX:+PrintFlagsFinal option is specified.

2) that -XX:+PrintFlagsFinal overrides earlier -XX:-PrintFlagsFinal.

3) that -XX:-PrintFlagsFinal overrides earlier -XX:+PrintFlagsFinal.

Fixes: eclipse-openj9#7764

Signed-off-by: Mark Stoodley <mstoodle@ca.ibm.com>
@keithc-ca
Copy link
Contributor

Jenkins test sanity win32,zlinux jdk8

@keithc-ca
Copy link
Contributor

I don't think we need to worry about the travis lint build failure:

wget -O bootjdk11.tar.gz https://api.adoptopenjdk.net/v3/binary/latest/11/ga/linux/x64/jdk/openj9/normal/adoptopenjdk
HTTP request sent, awaiting response... 503 Service Unavailable

@keithc-ca
Copy link
Contributor

Win32 testing failed due to #9707, but otherwise is good.

@keithc-ca keithc-ca merged commit 853d056 into eclipse-openj9:master May 27, 2020
@DanHeidinga
Copy link
Member

Enabling ElasticSearch on OpenJ9 is a big improvement for our project!
Adding my thanks to @mstoodle for starting this work and especially for his willingness to adapt the PR thru these discussions.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

NPE when launching Elasticsearch
4 participants