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.

Sign up for GitHub

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

Jump to bottom

Slow performance with imports in some cases #29162

Closed
wezell opened this issue Jul 8, 2024 · 4 comments · Fixed by #29169
Closed

Slow performance with imports in some cases #29162

wezell opened this issue Jul 8, 2024 · 4 comments · Fixed by #29169
Assignees
@erickgonzalez
Labels
OKR : Code Maintenance Owned by Erick QA : Approved Release : 23.10.24 v16 Included in LTS patch release 23.10.24 v16 Release : 24.04.24 v8 Included in LTS patch release 24.04.24 v8 Release : 24.09.21 UVE // Java21 Team : Bug Fixers Type : Defect Type : Enhancement

Comments

@wezell
Copy link
Contributor

wezell commented Jul 8, 2024

Parent Issue

No response

Problem Statement

We have seen some cases where importing content can take a long time. Additionally, when importing content, it spins up a new thread, which does not report to glowroot, making identifying these issues difficult. We need to optimize for this.

Steps to Reproduce

Create a content type with 20 fields. Run a csv import of a large amount of records for that type with glowroot attached to the dotCMS

There will be no glowroot reporting/traces to help identify slow code.

Acceptance Criteria

We can add some blind fixes here but the real test will be if we can get the import to work nicely in an environment that is sized down.

dotCMS Version

24.05

Proposed Objective

Application Performance

Proposed Priority

Priority 2 - Important

External Links... Slack Conversations, Support Tickets, Figma Designs, etc.

No response

Assumptions & Initiation Needs

No response

Quality Assurance Notes & Workarounds

No response

Sub-Tasks & Estimates

No response
wezell added a commit that referenced this issue Jul 9, 2024
@wezell
fix(imports) possible fix for imports running out of memory
7db9ed4 
ref: #29162
@wezell wezell mentioned this issue Jul 9, 2024
Merged
@wezell wezell linked a pull request Jul 9, 2024 that will close this issue
fix(imports) possible fix for imports running out of memory #29169
Merged
@wezell wezell self-assigned this Jul 9, 2024
@wezell wezell added the OKR : Code Maintenance Owned by Erick label Jul 9, 2024
wezell added a commit that referenced this issue Jul 9, 2024
@wezell
fix(imports) possible fix for imports running out of memory
446daec 
ref: #29162
wezell added a commit that referenced this issue Jul 9, 2024
@wezell
fix(imports) fix cache invalidations
ca82e17 
ref: #29162
wezell added a commit that referenced this issue Jul 9, 2024
@wezell
fix(imports) fix cache invalidations
71a2d41 
ref: #29162
@erickgonzalez erickgonzalez moved this from New to Current Sprint Backlog in dotCMS - Product Planning Jul 15, 2024
@erickgonzalez
Copy link
Contributor

erickgonzalez commented Jul 23, 2024 •
edited
Loading

Let's test how this branch issue-29162-slow-imports behaves against this Content Type and CSV.

Image

coursesCT-01J6DJJG7W1DC6NKJX8A7SNTV6.tar.gz
CSV-Courses.csv

It shouldn't throw a Java heap.

@erickgonzalez erickgonzalez assigned erickgonzalez and unassigned wezell Aug 6, 2024
erickgonzalez added a commit that referenced this issue Aug 27, 2024
@erickgonzalez
revert: revert unrequired changes ref: #29162
6845e2a
erickgonzalez added a commit that referenced this issue Aug 29, 2024
@erickgonzalez
fix: commitGranularity to 100 and copy CSV to tmp dir. ref: #29162
54b5993
erickgonzalez added a commit that referenced this issue Aug 29, 2024
@erickgonzalez
#29162 include in 24.04.24 LTS
94009f7
@erickgonzalez erickgonzalez added the LTS: Next Patch Shortlisted of issues that will be included in the upcoming LTS patch label Aug 29, 2024
@erickgonzalez
Copy link
Contributor

Tested Locally with -Xmx1G.

Master threw the following error before throwing the error imported 470 Courses:

Caused by: org.postgresql.util.PSQLException: ERROR: out of shared memory
2024-08-29 10:10:51   Hint: You might need to increase max_locks_per_transaction.

Branch imported 953 contentlets in 7 minutes and 6 seconds. (This is ok since the CSV has dupe courses).

16:18:20.442  INFO  action.ImportContentletsAction - -------- Starting Content Import Process --------

16:25:26.874  INFO  util.AdminLogger - UserId : dotcms.org.1 : class com.dotmarketing.portlets.contentlet.action.ImportContentletsAction : processAction : Importing Contentlets

@erickgonzalez erickgonzalez moved this from In Progress to In Review in dotCMS - Product Planning Aug 29, 2024
github-merge-queue bot pushed a commit that referenced this issue Aug 29, 2024
@wezell @erickgonzalez
fix(imports) possible fix for imports running out of memory (#29169)
0e6f1cf 
This does a few things:
- Removes reading the file in as a byte[] and instead passes around a
`File` object
- Changes how the import is run inline (helpful for glowroot traces) or
to use the DotSubmitter thread pool. Config var is
`IMPORT_CONTENTLETS_ASYNC`
- Commit Granularity changed to 100, meaning every 100 contentlets will
be committed and a new transaction will be started.
- Copied CSV file to tmp dir.



ref: #29162

---------

Co-authored-by: erickgonzalez <[email protected]>
@github-project-automation github-project-automation bot moved this from In Review to Internal QA in dotCMS - Product Planning Aug 29, 2024
erickgonzalez added a commit that referenced this issue Sep 4, 2024
@erickgonzalez
#29162 include in 23.10.24 LTS
b7fbe51
@gortiz-dotcms
Copy link
Contributor

gortiz-dotcms commented Sep 12, 2024 •
edited
Loading

Passed internal QA

Tested in demo, the import process lasted 7 minutes and 2 seconds

12:41:26:783 INFO action.ImportContentletsAction: -------- Starting Content Import Process -------- 
12:48:28:528   INFO util.AdminLogger: UserId : dotcms.org.1 : class com.dotmarketing.portlets.contentlet.action.ImportContentletsAction : processAction : Importing Contentlets

@erickgonzalez erickgonzalez added Release : 24.04.24 v8 Included in LTS patch release 24.04.24 v8 Release : 23.10.24 v16 Included in LTS patch release 23.10.24 v16 labels Sep 17, 2024
@josemejias11
Copy link
Contributor

Approved: Tested on trunk_5b70657, Docker, macOS 14.5, FF v126.0.1

Import took less than a minute to complete.

@josemejias11 josemejias11 moved this from QA - In Progress to Done in dotCMS - Product Planning Sep 19, 2024
@erickgonzalez erickgonzalez removed the LTS: Next Patch Shortlisted of issues that will be included in the upcoming LTS patch label Sep 19, 2024
@jgambarios jgambarios mentioned this issue Oct 30, 2024
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@erickgonzalez erickgonzalez

Labels
OKR : Code Maintenance Owned by Erick QA : Approved Release : 23.10.24 v16 Included in LTS patch release 23.10.24 v16 Release : 24.04.24 v8 Included in LTS patch release 24.04.24 v8 Release : 24.09.21 UVE // Java21 Team : Bug Fixers Type : Defect Type : Enhancement
Projects
dotCMS - Product Planning
Archived in project
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

fix(imports) possible fix for imports running out of memory dotCMS/core
4 participants
@wezell @erickgonzalez @josemejias11 @gortiz-dotcms