Skip to content

Optimize the search action #323

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

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

fantomitechno
Copy link

On big databases, the search action is very very slow .

On our 2 years running survival server, we waited more than 15 minutes for a /lg search source:fantomitechno with no answers.

We tested this PR on our big database and the same command now run in less than 10 seconds.

We replaced the multiple joins by checks on each Tables and matching the found indexes on the Action table.

@fantomitechno fantomitechno requested a review from a team as a code owner April 29, 2025 20:59
@DrexHD
Copy link
Contributor

DrexHD commented May 3, 2025

What database are you using to observe these differences? I believe when I looked into optimizing some of these queries I used sqlite, which properly optimized the join queries.

Ledger already has a cache for all "secondary" tables to optimize inserts. I have implemented a similar approach to this PR using these caches (to remove the select *s your PR has) at feat/remove-lookup-joins. Let me know if those changes would work for you!

@fantomitechno
Copy link
Author

What database are you using to observe these differences? I believe when I looked into optimizing some of these queries I used sqlite, which properly optimized the join queries.

We are using MySQL (especially MariaDB)
And someone else I talked too had the same problem with postgres.

Ledger already has a cache for all "secondary" tables to optimize inserts. I have implemented a similar approach to this PR using these caches (to remove the select *s your PR has) at feat/remove-lookup-joins. Let me know if those changes would work for you!

We will look into that and I'll come back to you!

Thank you for the answer (and sorry for the slow answer from me)

@DrexHD
Copy link
Contributor

DrexHD commented May 28, 2025

@fantomitechno did you look into this?

@fantomitechno
Copy link
Author

@DrexHD I didn't have time but I'm gonna test it tomorrow!

@fantomitechno
Copy link
Author

@fantomitechno did you look into this?

@DrexHD I was able to test and so I can confirm that it's as fast as my proposition!

@fantomitechno
Copy link
Author

After more usage, we encountered this error log that we didn't get with our PR nor with "normal" Ledger

[21:05:03] [DefaultDispatcher-worker-5/ERROR]: Uncaught exception in thread "DefaultDispatcher-worker-5"
java.lang.NullPointerException: null
        at knot/com.github.quiltservertools.ledger.database.DatabaseManager.getActionsFromQuery(DatabaseManager.kt:237) ~[ledger-1.3.10+build.941.jar:?]
        at knot/com.github.quiltservertools.ledger.database.DatabaseManager.selectActionsSearch(DatabaseManager.kt:510) ~[ledger-1.3.10+build.941.jar:?]
        at knot/com.github.quiltservertools.ledger.database.DatabaseManager.access$selectActionsSearch(DatabaseManager.kt:72) ~[ledger-1.3.10+build.941.jar:?]
        at knot/com.github.quiltservertools.ledger.database.DatabaseManager$searchActions$2.invokeSuspend(DatabaseManager.kt:153) ~[ledger-1.3.10+build.941.jar:?]
        at knot/com.github.quiltservertools.ledger.database.DatabaseManager$searchActions$2.invoke(DatabaseManager.kt) ~[ledger-1.3.10+build.941.jar:?]
        at knot/com.github.quiltservertools.ledger.database.DatabaseManager$searchActions$2.invoke(DatabaseManager.kt) ~[ledger-1.3.10+build.941.jar:?]
        at knot/com.github.quiltservertools.ledger.database.DatabaseManager$execute$2.invokeSuspend(DatabaseManager.kt:430) ~[ledger-1.3.10+build.941.jar:?]
        at knot/com.github.quiltservertools.ledger.database.DatabaseManager$execute$2.invoke(DatabaseManager.kt) ~[ledger-1.3.10+build.941.jar:?]
        at knot/com.github.quiltservertools.ledger.database.DatabaseManager$execute$2.invoke(DatabaseManager.kt) ~[ledger-1.3.10+build.941.jar:?]
        at knot/org.jetbrains.exposed.sql.transactions.experimental.SuspendedKt$suspendedTransactionAsyncInternal$1.invokeSuspend(Suspended.kt:193) ~[org_jetbrains_exposed_exposed-core-0.58.0-9e0dae56cfcfb508.jar:?]
        at knot/kotlin.coroutines.jvm.internal.BaseContinuationImpl.resumeWith(ContinuationImpl.kt:33) [org_jetbrains_kotlin_kotlin-stdlib-2.1.20-7cdd07a608b81ca8.jar:?]
        at knot/kotlinx.coroutines.DispatchedTask.run(DispatchedTask.kt:100) [org_jetbrains_kotlinx_kotlinx-co-jvm-1.10.1-e60f893de62e97b9.jar:?]
        at knot/kotlinx.coroutines.internal.LimitedDispatcher$Worker.run(LimitedDispatcher.kt:113) ~[org_jetbrains_kotlinx_kotlinx-co-jvm-1.10.1-e60f893de62e97b9.jar:?]
        at knot/kotlinx.coroutines.scheduling.TaskImpl.run(Tasks.kt:89) ~[org_jetbrains_kotlinx_kotlinx-co-jvm-1.10.1-e60f893de62e97b9.jar:?]
        at knot/kotlinx.coroutines.scheduling.CoroutineScheduler.runSafely(CoroutineScheduler.kt:586) [org_jetbrains_kotlinx_kotlinx-co-jvm-1.10.1-e60f893de62e97b9.jar:?]
        at knot/kotlinx.coroutines.scheduling.CoroutineScheduler$Worker.executeTask(CoroutineScheduler.kt:820) [org_jetbrains_kotlinx_kotlinx-co-jvm-1.10.1-e60f893de62e97b9.jar:?]
        at knot/kotlinx.coroutines.scheduling.CoroutineScheduler$Worker.runWorker(CoroutineScheduler.kt:717) [org_jetbrains_kotlinx_kotlinx-co-jvm-1.10.1-e60f893de62e97b9.jar:?]
        at knot/kotlinx.coroutines.scheduling.CoroutineScheduler$Worker.run(CoroutineScheduler.kt:704) [org_jetbrains_kotlinx_kotlinx-co-jvm-1.10.1-e60f893de62e97b9.jar:?]
        Suppressed: kotlinx.coroutines.internal.DiagnosticCoroutineContextException

The commands that created this issue were /lg search object:minecraft:end_crystal and /lg search world:minecraft:overworld

@DrexHD
Copy link
Contributor

DrexHD commented May 29, 2025

After more usage, we encountered this error log that we didn't get with our PR nor with "normal" Ledger

Should be fixed by f1fcd84!

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.

2 participants