https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36546 Julian Maurice <julian.maurice@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |julian.maurice@biblibre.com --- Comment #1 from Julian Maurice <julian.maurice@biblibre.com> --- Created attachment 164520 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=164520&action=edit Flamegraph of debian/templates/plack.psgi I also noticed that startup time was really long and ran debian/templates/plack.psgi with Devel::NYTProf to see what was happening. And indeed the YAML parsing/validation is what takes the most time (95% of the startup is spent into Koha::REST::V1::startup Here's the flamegraph to give an idea of the problem -- You are receiving this mail because: You are watching all bug changes. You are the assignee for the bug.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36546 Jonathan Druart <jonathan.druart@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- See Also| |https://bugs.koha-community | |.org/bugzilla3/show_bug.cgi | |?id=35613 -- You are receiving this mail because: You are the assignee for the bug. You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36546 --- Comment #2 from Tomás Cohen Arazi <tomascohen@gmail.com> --- Didier and I made some testing and bundling the spec reduces the startup time significantly (5x). -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36546 --- Comment #3 from Tomás Cohen Arazi <tomascohen@gmail.com> --- Created attachment 165412 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=165412&action=edit Bug 36546: Add a bundled version of the API spec This patch makes the Koha::REST::V1 class use a bundled version of the API spec. For that to happen, this patch also adds: * a `yarn api:bundle` task for generating the `swagger_bundle.yaml` file * the bundle file to `.gitignore` * @redocly/cli to the packages.json dependencies (yarn add @redocly/cli) * `api:bundle` is added to the `yarn build` general command NOTE: A new workflow is introduced by this patch as developers will need to call the `yarn api:bundle` command on each API spec change they make. To test: 1. Have KTD running 2. Verify the API works as usual => SUCCESS: The API just works 3. Apply this patch 4. Run: $ ktd --shell k$ yarn api:bundle 5. Restart plack: k$ koha-plack --restart kohadev ; tail -f /var/log/koha/kohadev/*.log => SUCCESS: The API loads correctly 6. Repeat 2 => SUCCESS: Nothing changed 7. Sign off :-D -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36546 David Cook <dcook@prosentient.com.au> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |dcook@prosentient.com.au --- Comment #5 from David Cook <dcook@prosentient.com.au> --- I am so happy to see other people talking about this, because this has been driving me crazy for *years*. It's arguably my biggest pet peeve about Koha! A few years ago, I got Jan Henning Thorsen to add "skip_validating_specification" to Mojolicious::Plugin::OpenAPI, and I've turned that on locally in Koha, but it hasn't made a noticeable difference to startup times, and when I use strace I notice something similar to Didier. The same syscalls over and over again: newfstatat(AT_FDCWD, "/usr", {st_mode=S_IFDIR|0755, st_size=4096, ...}, AT_SYMLINK_NOFOLLOW) = 0 newfstatat(AT_FDCWD, "/usr/share", {st_mode=S_IFDIR|0755, st_size=4096, ...}, AT_SYMLINK_NOFOLLOW) = 0 newfstatat(AT_FDCWD, "/usr/share/koha", {st_mode=S_IFDIR|0755, st_size=4096, ...}, AT_SYMLINK_NOFOLLOW) = 0 newfstatat(AT_FDCWD, "/usr/share/koha/api", {st_mode=S_IFDIR|0755, st_size=4096, ...}, AT_SYMLINK_NOFOLLOW) = 0 newfstatat(AT_FDCWD, "/usr/share/koha/api/v1", {st_mode=S_IFDIR|0755, st_size=4096, ...}, AT_SYMLINK_NOFOLLOW) = 0 newfstatat(AT_FDCWD, "/usr/share/koha/api/v1/swagger", {st_mode=S_IFDIR|0755, st_size=4096, ...}, AT_SYMLINK_NOFOLLOW) = 0 newfstatat(AT_FDCWD, "/usr/share/koha/api/v1/swagger/swagger.yaml", {st_mode=S_IFREG|0644, st_size=44125, ...}, AT_SYMLINK_NOFOLLOW) = 0 newfstatat(AT_FDCWD, "/usr/share/koha/api/v1/swagger/swagger.yaml", {st_mode=S_IFREG|0644, st_size=44125, ...}, 0) = 0 Jan was always skeptical about the validation being the problem, but he humoured me nonetheless. In my tests in 2021, skipping validation solved the problem, but using "skip_validating_specification" hasn't helped in 2024 (and earlier years). Even back then, I figured the problem was with the dereferencing of the spec, but it seemed to only happen with the validation. I'd be interested to see a flamegraph generated with Tomas's patch. I'm going to give it a go now and see what strace says... -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36546 --- Comment #7 from David Cook <dcook@prosentient.com.au> --- If I take down Koha and try to start it up, it breaks koha-testing-docker as it fails on the yarn api:bundle and exits. -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36546 --- Comment #9 from Tomás Cohen Arazi <tomascohen@gmail.com> --- (In reply to David Cook from comment #8)

rm -rf node_modules/ yarn install

Still doesn't work.

Trying to troubleshoot the issue, but it looks like it should be working when I look through the JS files.

It is weird, I didn’t manage to make it run before I left the office. If you want to try it, run it through npx instead -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36546 --- Comment #11 from David Cook <dcook@prosentient.com.au> --- (In reply to David Cook from comment #10)

Now I can get redocly to run, but it looks like this isn't going to work out of the box...

kohadev-koha@kohadevbox:koha(bug36546)$ redocly bundle api/v1/swagger/swagger.yaml > api/v1/swagger/swagger_bundle.yaml bundling api/v1/swagger/swagger.yaml... � Created a bundle for api/v1/swagger/swagger.yaml at stdout 422ms. -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36546 --- Comment #13 from David Cook <dcook@prosentient.com.au> --- (In reply to David Cook from comment #12)

Btw, I think the problem is that "js-yaml" is in "resolutions" in our package.json.

That's overriding the dependency for @redocly/openapi-core it seems.

Looks like Owen did that back in 2019 for bug 23025

Something to revisit it seems.

find node_modules -name "package.json" -exec grep 'js-yaml' {} \; "name": "@types/js-yaml", "description": "TypeScript definitions for js-yaml", "homepage": "https://github.com/DefinitelyTyped/DefinitelyTyped/tree/master/types/js-yaml", "directory": "types/js-yaml" "name": "js-yaml", "repository": "nodeca/js-yaml", "js-yaml": "bin/js-yaml.js" "module": "./dist/js-yaml.mjs", "import": "./dist/js-yaml.mjs", "unpkg": "dist/js-yaml.min.js", "jsdelivr": "dist/js-yaml.min.js", "js-yaml": "^4.1.0", "@types/js-yaml": "^4.0.3", "js-yaml": "^4.1.0", "name": "js-yaml", "homepage": "https://github.com/nodeca/js-yaml", "repository": "nodeca/js-yaml", "js-yaml": "bin/js-yaml.js" "unpkg": "dist/js-yaml.min.js", "jsdelivr": "dist/js-yaml.min.js", Probably time to upgrade our js-yaml version and stop the "resolutions" override... -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36546 David Cook <dcook@prosentient.com.au> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Failed QA |In Discussion -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36546 --- Comment #16 from David Cook <dcook@prosentient.com.au> --- I think perhaps we should use swagger.yml for development, and then we could build swagger_bundle.yml during the build. In koha-testing-docker, Koha is starting so fast I can't catch the Swagger parsing with strace. I'm excited to try this out on a test server with multiple instances and more realistic load than my dev machine. Great work here, Tomas (and everyone else)! -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36546 --- Comment #18 from Julian Maurice <julian.maurice@biblibre.com> --- (In reply to David Cook from comment #16)

I think perhaps we should use swagger.yml for development, and then we could build swagger_bundle.yml during the build.

I would say: use swagger_bundle.yml if it exists, fallback to swagger.yml if not. A slow startup is better than a failed one :) And using swagger_bundle.yml while developing something unrelated to the API would also improve the code/restart/test cycle speed -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36546 Paul Derscheid <paul.derscheid@lmscloud.de> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #165446|0 |1 is obsolete| | --- Comment #20 from Paul Derscheid <paul.derscheid@lmscloud.de> --- Created attachment 165450 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=165450&action=edit Bug 36546: Fix js-yaml dependency in package.json Signed-off-by: Paul Derscheid <paul.derscheid@lmscloud.de> -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36546 --- Comment #22 from Julian Maurice <julian.maurice@biblibre.com> --- (In reply to Tomás Cohen Arazi from comment #2)

Didier and I made some testing and bundling the spec reduces the startup time significantly (5x).

I see an improvement but not that significant. I ran the following command: time KOHA_HOME=$(pwd) perl debian/templates/plack.psgi On master, the average time is 3.5s With the patch, it's 2.4s The new flamegraph is not that much different, except that the `resolve` call takes less time relative to others Still a nice improvement! -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36546 Tomás Cohen Arazi <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|In Discussion |Signed Off -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36546 Tomás Cohen Arazi <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Summary|API yaml validation is |Bundle API spec to speed up |slow. |worker startup -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36546 --- Comment #26 from Paul Derscheid <paul.derscheid@lmscloud.de> --- Created attachment 165581 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=165581&action=edit Bug 36546: Use JSON instead Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Signed-off-by: Paul Derscheid <paul.derscheid@lmscloud.de> -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36546 David Cook <dcook@prosentient.com.au> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Signed Off |Failed QA --- Comment #28 from David Cook <dcook@prosentient.com.au> --- Failing this for now, since it's not deployable. 1. We need package.json to include yarn api:bundle in build:prod. 2. swagger_bundle.json will need to be added to the file_map in Makefile.PL as produced by build-resources.PL I think we should also include a fallback to swagger.yml if swagger_bundled.json is missing, although technically it should be installed by "make install", so I suppose it's not the end of the world... I can look at writing that missing patch... -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36546 --- Comment #29 from David Cook <dcook@prosentient.com.au> --- Created attachment 165821 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=165821&action=edit Bug 36546: Deploy swagger_bundle.json via make To test: 0. Apply patch 1a. perl Makefile.PL 1b. Choose "single" Installation mode 1c. Specify /opt/koha directory 2. make 2b. ls blib/API_CGI_DIR/v1/swagger/swagger_bundle.json 3. Ensure you're running as root or make koha user own /opt 4. make install 5. ls /opt/koha/api/v1/swagger/swagger_bundle.json -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36546 --- Comment #31 from David Cook <dcook@prosentient.com.au> --- (In reply to David Cook from comment #30)

Actually yeah I think we do need the failover to swagger.json or else many users of koha-testing-docker will have broken Koha instances...

Ah wait... my bad. ktd runs yarn build so you should have a api/v1/swagger/swagger_bundle.json Then any core API changes will require a rebuild. Not the end of the world... -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36546 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #165449|0 |1 is obsolete| | --- Comment #32 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Created attachment 165828 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=165828&action=edit Bug 36546: Add a bundled version of the API spec This patch makes the Koha::REST::V1 class use a bundled version of the API spec. For that to happen, this patch also adds: * a `yarn api:bundle` task for generating the `swagger_bundle.yaml` file * the bundle file to `.gitignore` * @redocly/cli to the packages.json dependencies (yarn add @redocly/cli) * `api:bundle` is added to the `yarn build` general command NOTE: A new workflow is introduced by this patch as developers will need to call the `yarn api:bundle` command on each API spec change they make. To test: 1. Have KTD running 2. Verify the API works as usual => SUCCESS: The API just works 3. Apply this patch 4. Run: $ ktd --shell k$ yarn api:bundle 5. Restart plack: k$ koha-plack --restart kohadev ; tail -f /var/log/koha/kohadev/*.log => SUCCESS: The API loads correctly 6. Repeat 2 => SUCCESS: Nothing changed 7. Sign off :-D Signed-off-by: Paul Derscheid <paul.derscheid@lmscloud.de> Signed-off-by: David Cook <dcook@prosentient.com.au> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36546 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #165589|0 |1 is obsolete| | --- Comment #34 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Created attachment 165830 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=165830&action=edit Bug 36546: Use JSON instead Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Signed-off-by: Paul Derscheid <paul.derscheid@lmscloud.de> Signed-off-by: David Cook <dcook@prosentient.com.au> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36546 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- QA Contact|testopia@bugs.koha-communit |martin.renvoize@ptfs-europe |y.org |.com -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36546 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Signed Off |Passed QA -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36546 --- Comment #37 from Katrin Fischer <katrin.fischer@bsz-bw.de> --- Pushed for 24.05! Well done everyone, thank you! -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36546 --- Comment #39 from Katrin Fischer <katrin.fischer@bsz-bw.de> --- When I run yarn:build locally it first complains about an outdated module and then leaves me with an untracked file: api/v1/swagger/swagger_bundle.json -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36546 --- Comment #41 from Tomás Cohen Arazi <tomascohen@gmail.com> --- (In reply to Katrin Fischer from comment #38)

I suspect this to have broken API driven things like patron search:

Something went wrong when loading the table. 404: Not Found.

HELP.

This is related to bug 36722. I was able to reproduce the failure, but at some point it started working again. So probably a networking issue with json-schema.org The right solution is on 36722. And should be done on packaging. -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36546 --- Comment #43 from Tomás Cohen Arazi <tomascohen@gmail.com> --- (In reply to David Cook from comment #28)

Failing this for now, since it's not deployable.

1. We need package.json to include yarn api:bundle in build:prod.

I'm having hard time understanding how we lost that line, which was included on the first patch! I don't see any patch removing it by itself! -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36546 --- Comment #45 from David Cook <dcook@prosentient.com.au> --- (In reply to Katrin Fischer from comment #39)

When I run yarn:build locally it first complains about an outdated module and then leaves me with an untracked file:

api/v1/swagger/swagger_bundle.json

You'll need to "rm -rf node_modules" and re-run "yarn install" to fix the outdated module issue. -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36546 --- Comment #47 from David Cook <dcook@prosentient.com.au> --- (At some point, it could be interesting to look at resolving the API spec in a module which could be pre-loaded like in bug 36721, because that could deliver even further performance gains.) -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36546 --- Comment #48 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Created attachment 165945 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=165945&action=edit Bug 36546: (follow-up) Add fallback to unbundled spec Some developers are running without ktd still and as such may not want to install the node modules required to bundle the specification. This patch adds a fallback to the unbundled yaml files when the bundle.json file isn't found. Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36546 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|ASSIGNED |Passed QA -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36546 Katrin Fischer <katrin.fischer@bsz-bw.de> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Passed QA |Pushed to main -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36546 --- Comment #51 from David Cook <dcook@prosentient.com.au> --- What are we thinking in terms of backporting this one? -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36546 Fridolin Somers <fridolin.somers@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |fridolin.somers@biblibre.co | |m Keywords|additional_work_needed | -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36546 Fridolin Somers <fridolin.somers@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC|didier.gautheron@biblibre.c | |om | -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36546 --- Comment #53 from David Cook <dcook@prosentient.com.au> --- (In reply to Fridolin Somers from comment #52)

(In reply to David Cook from comment #51)

...

What are we thinking in terms of backporting this one?

I'm on it. Looks like a change that may affect all API. Some non-packaged, non-debian install may be surprised about this change. For me this is not a change we should apply on stable branches.

In theory, non-packaged non-debian installs should be OK so long as they update using "make". Even for a git/dev install that doesn't, the fallback to the unbundled spec should still work. Let me know if I can help in any way :) -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36546 Phil Ringnalda <phil@chetcolibrary.org> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |phil@chetcolibrary.org Status|Pushed to main |Pushed to stable -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36546 David Cook <dcook@prosentient.com.au> changed: What |Removed |Added ---------------------------------------------------------------------------- See Also| |https://bugs.koha-community | |.org/bugzilla3/show_bug.cgi | |?id=37911 -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36546 David Cook <dcook@prosentient.com.au> changed: What |Removed |Added ---------------------------------------------------------------------------- Blocks| |39849 See Also|https://bugs.koha-community | |.org/bugzilla3/show_bug.cgi | |?id=39849 | Referenced Bugs: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=39849 [Bug 39849] Target dependency issue in Makefile -- You are receiving this mail because: You are watching all bug changes.