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

IPM 0.9 dependency resolution taking a long time #652

Closed
isc-shuliu opened this issue Dec 10, 2024 · 3 comments · Fixed by #656
Closed

IPM 0.9 dependency resolution taking a long time #652

isc-shuliu opened this issue Dec 10, 2024 · 3 comments · Fixed by #656
Assignees
Labels
bug Something isn't working prio: high regression-0.9 Regression bug introduced by IPM 0.9
Milestone

Comments

@isc-shuliu
Copy link
Collaborator

Some packages will sporadically take a long time to install on the v1 branch which has IPM 0.9.x.

Steps to Reproduce

To reproduce this issue:

  1. git clone the repo and switch to the v1 branch
  2. docker compose -d --build
  3. docker exec -it ipm-iris-1 bash
  4. Inside the container, run iris session iris
  5. In the IRIS session, run zpm "install -v demo-pandas-analytics"
  6. Observe the output, for me, it keeps hanging after the following output:
Module demo-pandas-analytics was downloaded from registry (https://pm.community.intersystems.com/)
Loading from /usr/irissys/mgr/.modules/USER/demo-pandas-analytics/1.0.0/

Load started on 12/10/2024 19:47:39
Loading file /usr/irissys/mgr/.modules/USER/demo-pandas-analytics/1.0.0/module.xml as xml
Imported document: demo-pandas-analytics.ZPM
Load finished successfully.

Dependencies:
dsw 3.2.26 @ registry
mdx2json 3.2.41 @ registry
yaml-utils 0.1.4 @ registry
zpm-registry 1.3.2 @ registry
  1. Press Ctrl+p followed by Ctrl + q to detach from the container.
  2. Run docker compose down && docker compose up -d --build && docker exec -it ipm-iris-1 /bin/bash to start a fresh container
  3. Run iris session iris to start an IRIS session
  4. In the IRIS session, run the following commands to install dependencies first
  • zpm "install -v dsw 3.2.26"
  • zpm "install -v mdx2json 3.2.41"
  • zpm "install -v yaml-utils 0.1.4"
  • zpm "install -v zpm-registry 1.3.2"
  1. Now zpm "install -v demo-pandas-analytics" finishes within a reasonable period of time.
@isc-shuliu isc-shuliu added bug Something isn't working prio: high labels Dec 10, 2024
@isc-shuliu isc-shuliu added this to the Backlog milestone Dec 10, 2024
@isc-shuliu
Copy link
Collaborator Author

A number of packages are influenced by this bug, including

  • demo-pandas-analytics
  • iris-flow
  • telegram-adapter-demo
  • openapi-suite
  • iris-flow

@isc-tleavitt isc-tleavitt self-assigned this Dec 10, 2024
@isc-tleavitt
Copy link
Contributor

This is a deadlock in %SYS.NAMESPACE resulting from 6ed90b1 - the logic here is very flawed, we explicitly add mappings for everything because of course the item doesn't exist yet, we're loading it for the first time.

This needs to be fixed for 0.9 release and may impact HS adoption.

cc @isc-kiyer @isc-eneil @isc-jlechtne @isc-jili

@isc-tleavitt isc-tleavitt added the regression-0.9 Regression bug introduced by IPM 0.9 label Dec 11, 2024
@isc-kiyer
Copy link
Collaborator

isc-kiyer commented Dec 11, 2024

@isc-tleavitt thanks for flagging that. We would not run into it for hscore since we use -DNoMapping=1 but would for other components. Also, the code should probably use %SYS.Namespace:Get*Dest methods to check if the item would be loaded into the expected database instead of already checking existence.

cc @isc-shuliu @isc-eneil @isc-jlechtne @isc-jili

isc-tleavitt added a commit that referenced this issue Dec 12, 2024
Fix #652: Don't create mappings unless they're actually needed
@github-project-automation github-project-automation bot moved this from To Do to Done in v0.9.0 release blockers Dec 12, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working prio: high regression-0.9 Regression bug introduced by IPM 0.9
Projects
Development

Successfully merging a pull request may close this issue.

3 participants