Skip to content

Conversation

@martijnvg
Copy link
Member

In the case a database couldn't be loaded, the geoip processor factory
checks whether any databases are available and then returns a processor
implementation that tags documents with the fact that required database
wasn't available. The GeoIpProcessor itself also loads the database, but
in case a database can't be loaded then it always fails with resource
missing exception. The GeoIpProcessor is modified in this change to
also check whether any database is available and in that case tag
documents instead of failing.

GeoIpDownloaderIT improvements:

  • The testUseGeoIpProcessorWithDownloadedDBs() was adding databases to config dirs,
    but not cleaning it up. Which broke assumptions in others in this suite, because
    the test cluster is reused.
  • Use the geoip stats api after each test to wait for a clean state, which means
    wait for database downloader to be disabled and all database files to be removed
    on all ingest nodes.
  • Don't use IngestDocument#getFieldValue(...) in test code surrounded by assertBusy(...).
    If a field isn't there an illegal state exception is thrown, which isn't caught by
    assertBusy(...). Only assertion errors are handled.

Closes #79074

In the case a database couldn't be loaded, the geoip processor factory
checks whether any databases are available and then returns a processor
implementation that tags documents with the fact that required database
wasn't available. The GeoIpProcessor itself also loads the database, but
in case a database can't be loaded then it always fails with resource
missing exception. The GeoIpProcessor is modified in this change to
also check whether any database is available and in that case tag
documents instead of failing.

GeoIpDownloaderIT improvements:
* The `testUseGeoIpProcessorWithDownloadedDBs()` was adding databases to config dirs,
  but not cleaning it up. Which broke assumptions in others in this suite, because
  the test cluster is reused.
* Use the geoip stats api after each test to wait for a clean state, which means
  wait for database downloader to be disabled and all database files to be removed
  on all ingest nodes.
* Don't use `IngestDocument#getFieldValue(...)` in test code surrounded by `assertBusy(...)`.
  If a field isn't there an illegal state exception is thrown, which isn't caught by
  `assertBusy(...)`. Only assertion errors are handled.

Closes elastic#79074
@martijnvg martijnvg added >non-issue :Data Management/Ingest Node Execution or management of Ingest Pipelines including GeoIP v8.0.0 labels Oct 14, 2021
@martijnvg martijnvg requested a review from probakowski October 14, 2021 09:14
@elasticmachine elasticmachine added the Team:Data Management Meta label for data/management team label Oct 14, 2021
@elasticmachine
Copy link
Collaborator

Pinging @elastic/es-data-management (Team:Data Management)

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 👍

@martijnvg martijnvg merged commit 5e18333 into elastic:master Oct 14, 2021
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 v8.0.0-beta1

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[CI] GeoIpDownloaderIT testStartWithNoDatabases failing

4 participants