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

path.data does not accept the legacy format with a comma-separated list for MDP anymore #76181

Closed
rsommer opened this issue Aug 5, 2021 · 10 comments · Fixed by #79272
Closed
Labels
>bug :Core/Infra/Core Core issues without another label Team:Core/Infra Meta label for core/infra team

Comments

@rsommer
Copy link

rsommer commented Aug 5, 2021

Elasticsearch version:
Version: 7.14.0, Build: default/deb/dd5a0a2acaa2045ff9624f3729fc8a6f40835aa1/2021-07-29T20:49:32.864135063Z, JVM: 16.0.1

JVM version:
openjdk version "11.0.12" 2021-07-20
OpenJDK Runtime Environment (build 11.0.12+7-post-Debian-2deb10u1)
OpenJDK 64-Bit Server VM (build 11.0.12+7-post-Debian-2deb10u1, mixed mode, sharing)

OS version:
4.19.0-14-amd64 #1 SMP Debian 4.19.171-2 (2021-01-30) x86_64 GNU/Linux

Description of the problem including expected versus actual behavior:
We upgraded a cluster from 7.13.4 to 7.14.0. This cluster still uses the deprecated MDP path.data-feature and the old config format using just a comma-separated list as config value for path.data

path.data: /path/to/disk01,/path/to/disk02

Restarting under 7.14.0 yields just one data dir, not both as under 7.13.4.

@rsommer rsommer added >bug needs:triage Requires assignment of a team area label labels Aug 5, 2021
@rsommer
Copy link
Author

rsommer commented Aug 5, 2021

As a sidenote, this configuration format was once a recommended workaround: https://www.elastic.co/blog/multi-data-path-bug-in-elasticsearch-5-3-0

@gwbrown gwbrown added :Core/Infra/Core Core issues without another label and removed needs:triage Requires assignment of a team area label labels Aug 5, 2021
@elasticmachine elasticmachine added the Team:Core/Infra Meta label for core/infra team label Aug 5, 2021
@elasticmachine
Copy link
Collaborator

Pinging @elastic/es-core-infra (Team:Core/Infra)

@gwbrown
Copy link
Contributor

gwbrown commented Aug 5, 2021

Relates to #71205.

@rjernst Looks like we might have broken this in 7.x - you might want to take a look, as I thought we didn't intend to remove support for this until 8.0.

@rsommer
Copy link
Author

rsommer commented Aug 5, 2021

MDP still works if using the (newer?) config notation:

path:
  data:
    - /path/to/disk01
    - /path/to/disk02

but still, this seems to be a regression between 7.13.4 and 7.14.0.

rjernst added a commit to rjernst/elasticsearch that referenced this issue Aug 5, 2021
For multiple data paths, there are several allowed varieties of
specifcying the paths. The yaml list forms, both single and multi line,
are allowed, but another form is a string containing commas that is
parsed as a list. This latter form was broken recently by the
refactoring of path.data parsing to emit deprecation warnings for MDP.
This commit fixes the comma separated string case and adds a test.

closes elastic#76181
@gboddin
Copy link

gboddin commented Aug 7, 2021

Can't believe this kind of changes makes it to stable.

Breaks infra on upgrade/install because in comma mode (,) elastic still starts and only uses the first element in the list.

rjernst added a commit that referenced this issue Aug 12, 2021
For multiple data paths, there are several allowed varieties of
specifcying the paths. The yaml list forms, both single and multi line,
are allowed, but another form is a string containing commas that is
parsed as a list. This latter form was broken recently by the
refactoring of path.data parsing to emit deprecation warnings for MDP.
This commit fixes the comma separated string case and adds a test.

closes #76181
rjernst added a commit that referenced this issue Aug 13, 2021
For multiple data paths, there are several allowed varieties of
specifcying the paths. The yaml list forms, both single and multi line,
are allowed, but another form is a string containing commas that is
parsed as a list. This latter form was broken recently by the
refactoring of path.data parsing to emit deprecation warnings for MDP.
This commit fixes the comma separated string case and adds a test.

closes #76181
@jeffkirk1
Copy link

I am experiencing the same problem, but I am using docker-compose.yml to supply the path.data definition, and the recommended YAML config for elasticsearch.yml won't work for me. I would greatly appreciate it if someone could supply a syntactically correct way to define this in docker-compose.yml.

@geekpete
Copy link
Member

If you're using a docker configuration then any entries underneath an environment section that will be used as environment variables need to be dot notation strings rather than a yaml list, in the form of key=value. For the path.data yaml list example above this needs to be a dot notation string for each list item, so you can use:

environment:
  - path.data.0=/path/to/disk01
  - path.data.1=/path/to/disk02

Also double check your spacing is correct when you update your configuration, especially indent/nesting levels/preceding spaces, as per standard yaml editing fun.

@jeffkirk1
Copy link

Thanks, geekpete, I was given similar info by Elastic support and my cluster is now back up and operational. I am not happy that this bug made it through testing, but I was very happy with Elastic's responsiveness and professionalism in finding the solution. I suffered no permanent data loss. Can't have a better outcome than that! Not so happy that I have to reconfigure my servers so I can't take advantage of distributing my I/O across my NVMe SSDs, but I can understand the reasons behind the multi data path feature being deprecated.

@rjernst
Copy link
Member

rjernst commented Aug 30, 2021

This issue was fixed by #76202 and should be out in 7.14.1

@rjernst rjernst closed this as completed Aug 30, 2021
@rsommer
Copy link
Author

rsommer commented Sep 6, 2021

Just rolled out 7.14.1, everything works as expected again. Thanks!

rjernst added a commit to rjernst/elasticsearch that referenced this issue Oct 15, 2021
For multiple data paths, there are several allowed varieties of
specifcying the paths. The yaml list forms, both single and multi line,
are allowed, but another form is a string containing commas that is
parsed as a list. This latter form was broken recently by the
refactoring of path.data parsing to emit deprecation warnings for MDP.
This commit fixes the comma separated string case and adds a test.

closes elastic#76181
rjernst added a commit that referenced this issue Oct 18, 2021
For multiple data paths, there are several allowed varieties of
specifcying the paths. The yaml list forms, both single and multi line,
are allowed, but another form is a string containing commas that is
parsed as a list. This latter form was broken recently by the
refactoring of path.data parsing to emit deprecation warnings for MDP.
This commit fixes the comma separated string case and adds a test.

closes #76181
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
>bug :Core/Infra/Core Core issues without another label Team:Core/Infra Meta label for core/infra team
Projects
None yet
Development

Successfully merging a pull request may close this issue.

7 participants