Add param and user setting to control elasticsearch behavior




2 years ago
2 years ago


(Reporter: dylan, Assigned: dylan)





(1 attachment)

44 bytes, text/x-github-pull-request
: review+
Details | Review | Splinter Review


2 years ago
Currently the search code relies on the elasticsearch_nodes parameter,
and the user matching uses error handling to fallback to the old search provider.

Both of those are good ideas, but we should short-circuit that logic when ES is intentionally disabled.

Comment 1

2 years ago
As currently defined, this would be a global localconfig option.

We can also implement this as a user preference, and user preferences can be globally turned off or on,
so that would accomplish the same ("turn off ES for all users" because it is down, for instance)

In general user-facing features should be protected behind feature flags (as we've done with HTML emails, Bug Modal, and so on)
but this particular feature is meant as a performance enhancement for the system.

1 user out of 10,000 opting out is fine, but if we start out as opt-in there will be less measurable 
performance difference because of the tragedy of the commons. If *your* search takes 1-2 minutes, that is 1-2 minutes
of a web worker (which we have a large but limited number of) cannot handle any other requests.

(this being a user preference also incurs a small but measurable performance impact, probably around 100ms).

I think this can play into planning and how we message the users on this change. We're not currently offering *better* search -- we're offering faster search (that hopefully, 99% of the time, returns the same results but quicker).


2 years ago
Summary: Add boolean to enable/disable elasticsearch → Add param and user setting to control elasticsearch behavior

Comment 2

2 years ago
Created attachment 8859734 [details] [review]

I really fretted about who to ask for review about this, my decision is based on the fact that glob's review time is a valuable resource and that this is not a piece of code that involves too much arcane BMO knowledge, but rather just a second set eyes. As an added precaution, it will receive extended testing on bugzilla-dev.
Attachment #8859734 - Flags: review?(rsoderberg)

Comment 3

2 years ago
I left a review at Github. The BMO specific stuff seemed concentrated in areas that will be exercised by elasticsearch testing on bugzilla-dev.

Comment 4

2 years ago
Comment on attachment 8859734 [details] [review]

See github for details.
Attachment #8859734 - Flags: review?(rsoderberg) → review+

Comment 5

2 years ago
   70709b3ac..12ea71539  elasticsearch -> elasticsearch
Last Resolved: 2 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.