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

Take the node id into account when creating geoip tmp dir. #70462

Merged
merged 3 commits into from
Mar 17, 2021

Conversation

martijnvg
Copy link
Member

This change adjust where the geoip tmp directory is created
to avoid issues when running multiple nodes on the same machine.

In the java tmp dir, a 'geoip-databases' directory is created and
directly under this directory a directory with the node id as name is created.
This allows safely running multiple nodes on the same machine (this
happens mainly during tests).

Closes #69972
Relates to #68920

This change adjust where the geoip tmp directory is created
to avoid issues when running multiple nodes on the same machine.

In the java tmp dir, a 'geoip-databases' directory is created and
directly under this directory a directory with the node id as name is created.
This allows safely running multiple nodes on the same machine (this
happens mainly during tests).

Closes elastic#69972
Relates to elastic#68920
@martijnvg martijnvg added >non-issue :Data Management/Ingest Node Execution or management of Ingest Pipelines including GeoIP v8.0.0 v7.13.0 labels Mar 16, 2021
@martijnvg martijnvg requested a review from probakowski March 16, 2021 14:54
@elasticmachine elasticmachine added the Team:Data Management Meta label for data/management team label Mar 16, 2021
@elasticmachine
Copy link
Collaborator

Pinging @elastic/es-core-features (Team:Core/Features)

Copy link
Contributor

@probakowski probakowski left a comment

Choose a reason for hiding this comment

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

LGTM, thanks @martijnvg! I left 1 minor suggestion

throw new UncheckedIOException(e);
}
}).filter(path -> {
return StreamSupport.stream(clusterService().state().nodes().getDataNodes().values().spliterator(), false)
Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe extract data nodes ids to local variable?

Set<String> ids = StreamSupport.stream(clusterService().state().nodes().getDataNodes().values().spliterator(), false)
    .map(c -> c.value.getId())
    .collect(Collectors.toSet());
final List<Path> geoipTmpDirs = ...
    }).filter(path -> ids.contains(path.getFileName().toString()))

@martijnvg martijnvg merged commit 3d3ec5c into elastic:master Mar 17, 2021
martijnvg added a commit to martijnvg/elasticsearch that referenced this pull request Mar 17, 2021
Backport elastic#70462 to 7.x branch.

This change adjust where the geoip tmp directory is created
to avoid issues when running multiple nodes on the same machine.

In the java tmp dir, a 'geoip-databases' directory is created and
directly under this directory a directory with the node id as name is created.
This allows safely running multiple nodes on the same machine (this
happens mainly during tests).

Closes elastic#69972
Relates to elastic#68920
martijnvg added a commit that referenced this pull request Mar 17, 2021
Backport #70462 to 7.x branch.

This change adjust where the geoip tmp directory is created
to avoid issues when running multiple nodes on the same machine.

In the java tmp dir, a 'geoip-databases' directory is created and
directly under this directory a directory with the node id as name is created.
This allows safely running multiple nodes on the same machine (this
happens mainly during tests).

Closes #69972
Relates to #68920
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
:Data Management/Ingest Node Execution or management of Ingest Pipelines including GeoIP >non-issue Team:Data Management Meta label for data/management team v7.13.0 v8.0.0-alpha1
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[CI] GeoIpDownloaderIT#testUseGeoIpProcessorWithDownloadedDBs() failure
4 participants