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

[Media Viewer] Performance on clicks is slow #179

Closed
Tracked by #790
mahalakshme opened this issue Oct 15, 2024 · 22 comments
Closed
Tracked by #790

[Media Viewer] Performance on clicks is slow #179

mahalakshme opened this issue Oct 15, 2024 · 22 comments
Assignees

Comments

@mahalakshme
Copy link

mahalakshme commented Oct 15, 2024

https://avni.freshdesk.com/a/tickets/4775

Issue:

On clicking MediaViewer and on clicking a image - take around 1 min to load.
Some users feel this used to take only around 5 seconds before

AC:

The below actions(basically any action in Media Viewer) should not take more than 5 secs to load

  • On click of 'Media Viewer' box in home page
  • On click of an image in Media Viewer
  • On selection of any filters and on clicking 'Apply filter' on Media Viewer
  • Validate the above performance improvement using gatling

Technical suggestions:

Image

  • Clicking on an image/media we need not again call /et/media/search? since we would already have the pre-signed image URLs to fetch or if it has got expired can fetch the pre-signed URL for that image alone.
  • On moving across each image on the carousel additional fetching of consecutive images can be done, instead of fetching everything in one go
  • When rendering the full page, pre-signing images link for thumbnails should suffice, instead of pre-signing original image link as well
@mahalakshme mahalakshme changed the title Faster media viewer load [Media Viewer] Performance on clicks is slow Oct 21, 2024
@1t5j0y 1t5j0y self-assigned this Oct 22, 2024
@1t5j0y 1t5j0y transferred this issue from avniproject/avni-webapp Oct 22, 2024
1t5j0y added a commit to avniproject/avni-etl that referenced this issue Oct 22, 2024
1t5j0y added a commit to avniproject/avni-etl that referenced this issue Oct 22, 2024
1t5j0y added a commit to avniproject/avni-etl that referenced this issue Oct 23, 2024
- Respect index configuration change on media table
- Use alternate query when media is not filtered based on address to avoid expensive address join for orgs with lot of address levels
@himeshr himeshr moved this from Code Review Ready to Code Review with Comments in Avni Product Oct 25, 2024
@himeshr himeshr moved this from Code Review with Comments to Code Review Ready in Avni Product Oct 25, 2024
@himeshr himeshr moved this from Code Review Ready to In Code Review in Avni Product Oct 25, 2024
@himeshr himeshr moved this from In Code Review to QA Ready in Avni Product Oct 25, 2024
@himeshr
Copy link
Contributor

himeshr commented Oct 25, 2024

We have satisfied the AC requirements except for writing gatling tests.. Not sure how we would do that as well.

@AchalaBelokar AchalaBelokar moved this from In QA to Done in Avni Product Oct 29, 2024
@AchalaBelokar AchalaBelokar moved this from In QA to Done in Avni Product Oct 29, 2024
@AchalaBelokar AchalaBelokar moved this from Done to QA Failed in Avni Product Oct 29, 2024
@AchalaBelokar
Copy link

  • the page number count is not showing in the lane of next and previous button

@1t5j0y
Copy link
Contributor

1t5j0y commented Oct 29, 2024

@AchalaBelokar reported issue is same as prod behaviour and not part of the AC of this card.

@1t5j0y 1t5j0y moved this from QA Failed to QA Ready in Avni Product Oct 29, 2024
@AchalaBelokar AchalaBelokar moved this from QA Ready to Done in Avni Product Oct 30, 2024
@mahalakshme
Copy link
Author

@1t5j0y still it seems to be slow as before, also page 1 call is going for page 0 itself. And clicking on an image also renders it very slowly, not sure if there was some issue in deployment.

@mahalakshme mahalakshme reopened this Nov 14, 2024
@github-project-automation github-project-automation bot moved this from Done to Triaged in Avni Product Nov 14, 2024
@mahalakshme mahalakshme moved this from Triaged to QA Failed in Avni Product Nov 14, 2024
@1t5j0y 1t5j0y moved this from QA Failed to In Progress in Avni Product Nov 15, 2024
1t5j0y added a commit to avniproject/avni-etl that referenced this issue Nov 15, 2024
@1t5j0y
Copy link
Contributor

1t5j0y commented Nov 15, 2024

Bug was introduced into ETL where new indexes are created for the same columns on media table on each run. 56 orgs currently have 100+ indexes on the corresponding 'media' tables.

Query to check impact:

select tm.schema_name, tm.name, count(*)from index_metadata im join table_metadata tm on im.table_metadata_id = tm.id
where im.column_id is null
group by tm.schema_name, tm.name;

Steps for tech QA

  1. select an affected org for which you are testing
  2. delete from index_metadata where table_metadata_id = (select id from table_metadata where schema_name = '<affected_org_schema_name>' and name = 'media');
  3. drop all indexes on media table for org you are testing against (can do this easily in intellij ui by navigating to schema -> 'media' table -> indexes -> right click -> drop)
  4. run etl couple of times for the affected org to check that indexes are recreated and not repeated

These steps will be repeated on prod for all affected orgs after QA pass and deployment of fix.

@mahalakshme
Copy link
Author

@1t5j0y forgot to add the screen recording(took yesterday) for the issue I noticed. Adding for reference to help the QA as well: https://drive.google.com/file/d/1w5q-OCYu8z-BVPIAhnLN-v6EL5NNXfOS/view?usp=drive_link

@himeshr
Copy link
Contributor

himeshr commented Nov 15, 2024

Testing results

Validated that the fix works as expected, without creating duplicates.

SQL to bulk drop all the indexes from media table and index_metadata(media) are as follows, the next etl run should only create 1 set of required indexes

select 'drop index "' || tm.schema_name|| '"."' || im.name || '";' from public.index_metadata im
join table_metadata tm on im.table_metadata_id = tm.id
where tm.name = 'media';

delete from index_metadata where table_metadata_id = (select id from table_metadata where name = 'media');

@1t5j0y
Copy link
Contributor

1t5j0y commented Nov 20, 2024

/etl/media/search was slow despite changes to query with improved plans and several query styles did not fix it. Simple count query on goonj.address was taking a long time which indicated that the query planner had bad statistics.
Ran (on prerelease and prod):

reindex (verbose) table goonj.address;
reindex (verbose) table goonj.media;
VACUUM (FULL, ANALYSE) goonj.address;

and query performance is now much better.
Created avniproject/avni-etl#114 to run this in a scheduled manner.

Additionally, optimised resource fetching from media client.

@1t5j0y 1t5j0y moved this from In Progress to Code Review Ready in Avni Product Nov 20, 2024
@AchalaBelokar
Copy link

  • I Login with maha@gvamp I created a activity with name Gaj which is not reflected on media viewer.. Not showing the images also ..but data entry app it is showing. ... platform : Prerelease

@AchalaBelokar AchalaBelokar moved this from In QA to QA Failed in Avni Product Nov 22, 2024
@himeshr
Copy link
Contributor

himeshr commented Nov 22, 2024

@AchalaBelokar

  • I Login with maha@gvamp I created a activity with name Gaj which is not reflected on media viewer.. Not showing the images also ..but data entry app it is showing. ... platform : Prerelease

Please note that i have configured prerelease ETL to use a different DB than prerelease avni server, to test the postgres upgradation..
Therefore you'll not see recent entities data there as of now..

To unblock your testing, we can revert the above change.

@mahalakshme
Copy link
Author

@himeshr yeah releasing 10.1.1 cards are higher priority, so we can do the needful asap to unblock the testing since it will help in testing fixes for 3 tickets.

@1t5j0y
Copy link
Contributor

1t5j0y commented Nov 22, 2024

@himeshr has already reverted the change. ETL for gvamp_uat is failing on prerelease.

Caused by: org.springframework.jdbc.BadSqlGrammarException: StatementCallback; bad SQL grammar [create table "gvamp_uat".village_village_form ("id" integer, "uuid" text, "is_voided" boolean, "created_by_id" integer, "last_modified_by_id" integer, "created_date_time" timestamp with time zone, "last_modified_date_time" timestamp with time zone, "organisation_id" integer, "name" text, "individual_id" integer, "address_id" integer, "earliest_visit_date_time" timestamp with time zone, "max_visit_date_time" timestamp with time zone, "encounter_date_time" timestamp with time zone, "encounter_location" point, "cancel_date_time" timestamp with time zone, "cancel_location" point, "legacy_id" text, "latest_approval_status" text, "Account  name" text, "Total Households" numeric, "Types of communities living" text, "Main Occupation of the Village" text, "Number of Male residents" numeric, "TotalVillagePopulation" numeric, "Number of Female Residents" numeric, "Description of the communities" text, "Center / Field office" text, "Account  name" text );
.
.
Caused by: org.postgresql.util.PSQLException: ERROR: column "Account  name" specified more than once

Have asked @AchalaBelokar to test with another org.

@1t5j0y 1t5j0y moved this from QA Failed to QA Ready in Avni Product Nov 22, 2024
@dinesh2096
Copy link

@1t5j0y we have created the work order and the uuid is '42680a41-c290-481c-9f58-a9946c74c9b6' we can able to see the data in DB but not in ETL table so the image we have added is not reflecting in the media viewer
env : per-release
org : rwb24uat
user : dinesh@rwb24uat

@dinesh2096 dinesh2096 moved this from In QA to QA Failed in Avni Product Nov 25, 2024
@1t5j0y
Copy link
Contributor

1t5j0y commented Nov 25, 2024

Images are visible in media viewer.
Image

@1t5j0y 1t5j0y moved this from QA Failed to QA Ready in Avni Product Nov 25, 2024
@dinesh2096 dinesh2096 self-assigned this Nov 26, 2024
@dinesh2096
Copy link

QA Reference :

Click here to watch the video

@dinesh2096 dinesh2096 moved this from In QA to Done in Avni Product Nov 26, 2024
@mahalakshme
Copy link
Author

mahalakshme commented Dec 9, 2024

I still find the media search is slow when loading the first page(taking 36 secs). Few days back it was faster. Few reasons I see:

  • [Media Viewer] Performance on clicks is slow #179 (comment) - Why call to 1st page is going when on page 0
  • Is another reason because we have not run reindex periodically? I dont see that can be an issue since we dont delete and only add new rows during which reindex will automatically happen. Am I missing something?

@mahalakshme mahalakshme reopened this Dec 9, 2024
@github-project-automation github-project-automation bot moved this from Done to Triaged in Avni Product Dec 9, 2024
@mahalakshme mahalakshme moved this from Triaged to QA Failed in Avni Product Dec 9, 2024
@mahalakshme
Copy link
Author

@1t5j0y

@1t5j0y 1t5j0y moved this from QA Failed to In Progress in Avni Product Dec 9, 2024
@1t5j0y
Copy link
Contributor

1t5j0y commented Dec 9, 2024

@mahalakshme doesn't look like the fix has been deployed to prod.

https://app.circleci.com/pipelines/github/avniproject/avni-media?branch=10.1

@mahalakshme
Copy link
Author

@1t5j0y looks like got missed - I thought as part of the fix for duplicate indices creation it was deployed so didnt add it to 10.1.1 card. Can you deploy it - add tags, update release notes and merge as needed as part of this card itself since release 10.1.1 is also done.

@mahalakshme mahalakshme self-assigned this Dec 9, 2024
@mahalakshme
Copy link
Author

deployed the changes - did tagging and merging - now testing

@mahalakshme
Copy link
Author

mahalakshme commented Dec 9, 2024

@1t5j0y fetching a page still takes 15-20 secs. Can anymore performance improvement can be made?

@1t5j0y
Copy link
Contributor

1t5j0y commented Dec 10, 2024

Seeing better performance (1-2s) again after running
VACUUM (FULL, ANALYSE, VERBOSE ) goonj.address;
and there is steady deterioration as more ETL runs occur.

Unsure of why this is making such a big difference as auto vacuum seems to be occurring regularly.

Not making any further changes on this card as we already have avniproject/avni-etl#114

@1t5j0y 1t5j0y closed this as completed Dec 10, 2024
@github-project-automation github-project-automation bot moved this from In Progress to Done in Avni Product Dec 10, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Archived in project
Development

No branches or pull requests

6 participants