Closed Bug 1348380 Opened 4 years ago Closed 4 years ago

Add param and user setting to control elasticsearch behavior


( :: General, enhancement, P1)






(Reporter: dylan, Assigned: dylan)




(1 file)

44 bytes, text/x-github-pull-request
: review+
Details | Review
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.
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).
Summary: Add boolean to enable/disable elasticsearch → Add param and user setting to control elasticsearch behavior
Attached file PR
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)
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 on attachment 8859734 [details] [review]

See github for details.
Attachment #8859734 - Flags: review?(rsoderberg) → review+
   70709b3ac..12ea71539  elasticsearch -> elasticsearch
Closed: 4 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.