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

replace custom calls with elasticsearch client gem #91

Open
wants to merge 6 commits into
base: master
Choose a base branch
from

Conversation

nvdk
Copy link
Member

@nvdk nvdk commented Nov 8, 2024

rationale: Moving to the elasticsearch client removes complexity from our own
client (less maintenance) and allows us to benefit from features offered by the
client.
Specifically:

  • full elasticsearch api support, which should allow us to implement new
    features more quickly (like metrics, updating index settings without a complete rebuild, ...)
  • the ability to set up tracing and logging of http calls
  • the ability to use connection pooling and persistent connections

Has been tested on the octopus app and seems to be working as expected, but needs further testing.
Confirmed working:

  • search using POST to /:path/search
  • reindexing
  • updates of attributes coming through
  • count queries

has performance improvements because of better http connection management, needs to be tested under heavy delta load.
NOTE: requires latest version of mu-jruby-template, make sure to pull that when testing/building locally

@nvdk nvdk requested review from madnificent and erikap November 8, 2024 11:05
@nvdk nvdk marked this pull request as draft November 8, 2024 13:45
@nvdk
Copy link
Member Author

nvdk commented Nov 8, 2024

There was an issue with webrick throwing "invalid URI" for every requests. Took some debugging, but found it. this is related to ruby/webrick#144 . however we can't override the webrick dependency here version, so requires a change in the template

@nvdk nvdk force-pushed the feature/elasticsearch-client branch from 9375a43 to 9654265 Compare November 8, 2024 15:08
nvdk added 4 commits November 8, 2024 16:21
rationale: Moving to the elasticsearch client removes complexity from our own
client (less maintenance) and allows us to benefit from features offered by the
client.
Specifically:
- full elasticsearch api support, which should allow us to implement new
features more quickly (like metrics, updating index settings without a complete
rebuild, ...)
- the ability to set up tracing and logging of http calls
- the ability to use connection pooling and persistent connections
faraday can use several http backends for its requests, quick research indicated
this to be the most used modern one. it should be easy to switch to one of the
other adapters (net http persistent or patron) if this one is not performant enough

sources:
- https://apisyouwonthate.com/blog/ruby-users-be-wary-of-net-http/
- https://www.elastic.co/guide/en/elasticsearch/client/ruby-api/current/troubleshooting.html#ruby-ts-adapter
- https://ruby.libhunt.com/compare-patron-vs-typhoeus
- elastic/elasticsearch-ruby#437
use latest template which has a newer webrick version
@nvdk nvdk force-pushed the feature/elasticsearch-client branch from 9654265 to 1996d56 Compare November 8, 2024 15:21
@nvdk nvdk marked this pull request as ready for review November 8, 2024 15:23
@nvdk
Copy link
Member Author

nvdk commented Nov 8, 2024

should be ready for testing now, seems to work as expected in the octopus app. needs further testing

nvdk and others added 2 commits November 8, 2024 16:26
this just causes extra confusion when debugging dependencies
it's now provided by the ruby template
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant