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

[ML] Take more care that normalize processes use unique named pipes #54636

Conversation

droberts195
Copy link
Contributor

When one of ML's normalize processes fails to connect to the JVM
quickly enough and another normalize process for the same job
starts shortly afterwards it is possible that their named pipes
can get mixed up.

This change avoids this risk by adding an incrementing counter
value into the named pipe names used for normalize processes.

Fixes #43830

When one of ML's normalize processes fails to connect to the JVM
quickly enough and another normalize process for the same job
starts shortly afterwards it is possible that their named pipes
can get mixed up.

This change avoids the risk of that by adding an incrementing
counter value into the named pipe names used for normalize
processes.

Fixes elastic#43830
@elasticmachine
Copy link
Collaborator

Pinging @elastic/ml-core (:ml)

@droberts195 droberts195 merged commit de93853 into elastic:master Apr 2, 2020
@droberts195 droberts195 deleted the extra_uniqueness_for_normalize_named_pipes branch April 2, 2020 12:15
droberts195 added a commit to droberts195/elasticsearch that referenced this pull request Apr 2, 2020
When one of ML's normalize processes fails to connect to the JVM
quickly enough and another normalize process for the same job
starts shortly afterwards it is possible that their named pipes
can get mixed up.

This change avoids the risk of that by adding an incrementing
counter value into the named pipe names used for normalize
processes.

Backport of elastic#54636
droberts195 added a commit to droberts195/elasticsearch that referenced this pull request Apr 2, 2020
When one of ML's normalize processes fails to connect to the JVM
quickly enough and another normalize process for the same job
starts shortly afterwards it is possible that their named pipes
can get mixed up.

This change avoids the risk of that by adding an incrementing
counter value into the named pipe names used for normalize
processes.

Backport of elastic#54636
@droberts195 droberts195 added v7.7.0 and removed v7.7.1 labels Apr 2, 2020
droberts195 added a commit that referenced this pull request Apr 2, 2020
…54641)

When one of ML's normalize processes fails to connect to the JVM
quickly enough and another normalize process for the same job
starts shortly afterwards it is possible that their named pipes
can get mixed up.

This change avoids the risk of that by adding an incrementing
counter value into the named pipe names used for normalize
processes.

Backport of #54636
droberts195 added a commit that referenced this pull request Apr 2, 2020
…54642)

When one of ML's normalize processes fails to connect to the JVM
quickly enough and another normalize process for the same job
starts shortly afterwards it is possible that their named pipes
can get mixed up.

This change avoids the risk of that by adding an incrementing
counter value into the named pipe names used for normalize
processes.

Backport of #54636
droberts195 added a commit to droberts195/elasticsearch that referenced this pull request Oct 19, 2020
The changes of elastic#54636 and elastic#60395 were incorrect in their assertion
that "the job ID passed to the process pipes is only used to make
the file names unique".  In fact it is also passed to the C++ log
handler and gets logged with every message logged by the C++
processes.

This PR splits the job ID and unique IDs (added in elastic#54636 and elastic#60395)
so that the correct job ID is passed to the log handler.

Nothing really bad happened as a result of this problem - it was
just cosmetic.
droberts195 added a commit that referenced this pull request Oct 19, 2020
The changes of #54636 and #60395 were incorrect in their assertion
that "the job ID passed to the process pipes is only used to make
the file names unique".  In fact it is also passed to the C++ log
handler and gets logged with every message logged by the C++
processes.

This PR splits the job ID and unique IDs (added in #54636 and #60395)
so that the correct job ID is passed to the log handler.

Nothing really bad happened as a result of this problem - it was
just cosmetic.
droberts195 added a commit that referenced this pull request Oct 19, 2020
The changes of #54636 and #60395 were incorrect in their assertion
that "the job ID passed to the process pipes is only used to make
the file names unique".  In fact it is also passed to the C++ log
handler and gets logged with every message logged by the C++
processes.

This PR splits the job ID and unique IDs (added in #54636 and #60395)
so that the correct job ID is passed to the log handler.

Nothing really bad happened as a result of this problem - it was
just cosmetic.
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.

[ML] Possible corrupt write to the normalizer process
4 participants