Skip to content

Search: Add hooks for failed attempts#13683

Merged
kraftbj merged 5 commits intomasterfrom
add/search-fail-hooks
Oct 7, 2019
Merged

Search: Add hooks for failed attempts#13683
kraftbj merged 5 commits intomasterfrom
add/search-fail-hooks

Conversation

@kraftbj
Copy link
Copy Markdown
Contributor

@kraftbj kraftbj commented Oct 7, 2019

When Search fails for some reason, it'll fall back to MySQL searching which can be very heavy for a database when ES results are assumed.

Adding some hooks to better be able to track where something comes off the rails and need to fall back to MySQL.

Changes proposed in this Pull Request:

  • Adds hooks for error handling purposes.

Is this a new feature or does it add/remove features to an existing part of Jetpack?

  • n/a

Testing instructions:

  • Would be used with something like
function custom_function ( $reason, $data = null ) {
	$logging = new CustomLogger;
	$logging->set_reason( $reason );
	if ( $data ) {
		$logging->set_additional_data( $data );
	}
	$logging->save();
}

Proposed changelog entry for your changes:

  • Search: Add hooks for when Search falls back to using the local database.

cc: @GaryJones

@kraftbj kraftbj added Enhancement Changes to an existing feature — removing, adding, or changing parts of it [Status] Needs Review This PR is ready for review. [Feature] Search For all things related to Search labels Oct 7, 2019
@kraftbj kraftbj added this to the 7.9 milestone Oct 7, 2019
@kraftbj kraftbj requested a review from a team October 7, 2019 16:56
jeherve
jeherve requested changes Oct 7, 2019
View reviewed changes
@jetpackbot
Copy link
Copy Markdown
Collaborator

jetpackbot commented Oct 7, 2019 •
edited
Loading

Thank you for the great PR description!

When this PR is ready for review, please apply the [Status] Needs Review label. If you are an a11n, please have someone from your team review the code if possible. The Jetpack team will also review this PR and merge it to be included in the next Jetpack release.

Scheduled Jetpack release: November 5, 2019.
Scheduled code freeze: October 29, 2019

Generated by 🚫 dangerJS against 5b1510d

@jeherve jeherve added [Status] Needs Author Reply We need more details from you. This label will be auto-added until the PR meets all requirements. and removed [Status] Needs Review This PR is ready for review. labels Oct 7, 2019
kraftbj and others added 4 commits October 7, 2019 12:00
@kraftbj @jeherve
Update modules/search/class.jetpack-search.php
e91b266 
Co-Authored-By: Jeremy Herve <jeremy@jeremy.hu>
@kraftbj @jeherve
Update modules/search/class.jetpack-search.php
0f17e29 
Co-Authored-By: Jeremy Herve <jeremy@jeremy.hu>
@kraftbj @jeherve
Update modules/search/class.jetpack-search.php
2384fce 
Co-Authored-By: Jeremy Herve <jeremy@jeremy.hu>
@kraftbj
Update docs
5b1510d
@kraftbj kraftbj added [Status] Needs Review This PR is ready for review. and removed [Status] Needs Author Reply We need more details from you. This label will be auto-added until the PR meets all requirements. labels Oct 7, 2019
@jeherve jeherve added [Status] Ready to Merge Go ahead, you can push that green button! and removed [Status] Needs Review This PR is ready for review. labels Oct 7, 2019
@matticbot
Copy link
Copy Markdown
Contributor

matticbot commented Oct 7, 2019

Caution: This PR has changes that must be merged to WordPress.com
Hello kraftbj! These changes need to be synced to WordPress.com - If you 're an a11n, please commandeer, review, and approve D33671-code before merging this PR. Thank you!

@kraftbj kraftbj merged commit e1b9a60 into master Oct 7, 2019
@kraftbj kraftbj deleted the add/search-fail-hooks branch October 7, 2019 17:17
@matticbot matticbot added [Status] Needs Changelog and removed [Status] Ready to Merge Go ahead, you can push that green button! labels Oct 7, 2019
@GaryJones
Copy link
Copy Markdown
Contributor

GaryJones commented Oct 8, 2019

Thanks @kraftbj!

jeherve added a commit that referenced this pull request Oct 23, 2019
@jeherve
changelog: add #13683
5721331
jeherve added a commit that referenced this pull request Oct 29, 2019
@jeherve
7.9: Changelog (#13608)
6cd1621 
* 7.9: Changelog

* Update version number

* Update stable tag and tested up to

* Changelog: add #13530

* changelog: add #13578

* Changelog: add #13598

* Changelog: add entry for numerous block preview changes

* Changelog: add #13599

* changelog: add #13541

* Changelog: add #13542

* Changelog: add #13331

* Changelog: add #13558

* Changelog: add #13409

* Changelog: add #13582

* Changelog: add #13600

* Changelog: add #13601

* Changelog: add #13595

* Changelog: add #12695

* Changelog: add #13009

* Changelog: add #13649

* Changelog: add #13450

* Changelog: add #13507

* Changelog: add #13658

* Changelog: add #13687

* changelog: add #13683

* Changelog: add #9323

* Changelog: add #13681

* Fix typos in readme

* Add link to WordPress Beta Tester plugin

* Changelog: add #13630

* Changelog: add #13695

* Changelog: add #13659

* Changelog: add #13716

* Changelog: add #13664

* Changelog: add #13682

* Changelog: add #13362

* Changelog: add #13563

* Add testing list for #13563

* Changelog: add #13735

* Changelog: add #13752

* Changelog: add #13624

* Changelog: add #13756

* Changelog: add #13745

* Changelog: add #13728

* Changelog: add #13779

* Changelog: add #13699

* Changelog: add #13804

* Changelog: add #13761

* Changelog: add #13637

* Changelog: add #13517

* Changelog: add #13521

* Changelog: add #13729

* Testing list: add testing instructions for #13729

* Changelog: add sync changes

* Changelog: add #13807

* Changelog: add #13654

* Changelog: add #13795

* Changelog: add #13801

* Changelog: add #13818

* Changelog: add #13725

* Changelog: add #13831

* Changelog: add #13516

* Testing list: add Twenty Twenty instructions

* Changelog: add #13799

* Changelog: add #13805

* Changelog: add #13688

* Changelog: add #13830
gibrown pushed a commit that referenced this pull request Nov 8, 2019
@kraftbj @gibrown
Search: Add hooks for failed attempts (jetpack:13683)
755c157 
Summary:
 When Search fails for some reason, it'll fall back to MySQL searching which can be very heavy for a database when ES results are assumed.

 Adding some hooks to better be able to track where something comes off the rails and need to fall back to MySQL.

 #### Changes proposed in this Pull Request:
 * Adds hooks for error handling purposes.

 #### Is this a new feature or does it add/remove features to an existing part of Jetpack?
 * n/a

 #### Testing instructions:
 * Would be used with something like
 ```add_action( 'jetpack_search_abort, 'custon_function', 10, 4 );
 function custom_function ( $reason, $data = null ) {
 	$logging = new CustomLogger;
 	$logging->set_reason( $reason );
 	if ( $data ) {
 		$logging->set_additional_data( $data );
 	}
 	$logging->save();
 }
 ```

 #### Proposed changelog entry for your changes:
 * Search: Add hooks for when Search falls back to using the local database.

 cc: @GaryJones

--

Automatically created by Jetpack Fusion from a Pull Request:
#13683

Test Plan: - [ ] Dummy test plan.

Reviewers: github-phab-bot

Subscribers: kraftbj

Tags: #touches_jetpack_files

Differential Revision: https://[private link]

Merges r197555-wpcom.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@jeherve jeherve jeherve approved these changes

Assignees

@kraftbj kraftbj

Labels

Enhancement Changes to an existing feature — removing, adding, or changing parts of it [Feature] Search For all things related to Search Touches WP.com Files

Projects

None yet

Milestone

7.9

Development

Successfully merging this pull request may close these issues.

5 participants

@kraftbj @jetpackbot @matticbot @GaryJones @jeherve