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

Optimize full-text to selectively rebuild #7610

Merged
merged 3 commits into from Mar 15, 2024

Conversation

Hydrocharged
Copy link
Contributor

@Hydrocharged Hydrocharged commented Mar 13, 2024

This changes the Full-Text rebuild process to only trigger on tables that were changed in both roots (ours and theirs), rather than rebuilding every time a merge is performed regardless of what was actually changed. The rules here are fairly permissive, in that if a parent table or any of the indexed child tables are changed in both, then we rebuild all Full-Text indexes for that table. I chose this behavior out of caution, as dual changes in a single child table is probably due to some kind of renaming, so it's safer to rebuild everything for that table.

@coffeegoddd
Copy link
Contributor

@Hydrocharged DOLT

comparing_percentages
100.000000 to 100.000000
version result total
d36505d ok 5937457
version total_tests
d36505d 5937457
correctness_percentage
100.0

@timsehn
Copy link
Sponsor Contributor

timsehn commented Mar 13, 2024

This change still causes mediawiki to hang if:

  1. An import job is running changing a table named searchindex with fulltext indexes
    -AND-
  2. You hit the website updating the objectcache and job tables which do not have fulltext indexes

Is that behavior intended?

@Hydrocharged
Copy link
Contributor Author

PR has been reworked to address the problem as described. I replicated the issue on the mediawiki dataset, and tested this PR's branch to confirm that it resolves the issue.

@coffeegoddd
Copy link
Contributor

@Hydrocharged DOLT

comparing_percentages
100.000000 to 100.000000
version result total
ee736b0 ok 5937457
version total_tests
ee736b0 5937457
correctness_percentage
100.0

@timsehn
Copy link
Sponsor Contributor

timsehn commented Mar 14, 2024

I tested this and it now works to import a dump and browse MediaWiki at the same time.

…refactor some things into functions, even if they are still too big.
Copy link
Contributor

@reltuk reltuk left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM! I opened a PR against this branch with some suggested simple refactorings (move things into functions) and some comments which made things clearer to me. Feel free to take it or leave it!

#7617

go/libraries/doltcore/merge: fulltext_rebuild.go: Add some comments, refactor some things into functions, even if they are still too big.
@coffeegoddd
Copy link
Contributor

@Hydrocharged DOLT

comparing_percentages
100.000000 to 100.000000
version result total
dcb2c7d ok 5937457
version total_tests
dcb2c7d 5937457
correctness_percentage
100.0

@Hydrocharged Hydrocharged merged commit 94df749 into main Mar 15, 2024
20 checks passed
@Hydrocharged Hydrocharged deleted the daylon/fulltext-optimization branch March 15, 2024 06:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants