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

[Task]: Page fetch getting called twice in consolidated view api #36243

Closed
sneha122 opened this issue Sep 11, 2024 · 0 comments · Fixed by #36247
Closed

[Task]: Page fetch getting called twice in consolidated view api #36243

sneha122 opened this issue Sep 11, 2024 · 0 comments · Fixed by #36247
Assignees
Labels
Integrations Pod General Issues related to the Integrations Pod that don't fit into other tags. Query & JS Pod Issues related to the query & JS Pod Task A simple Todo

Comments

@sneha122
Copy link
Contributor

SubTasks

there may be a corner case where the page document can get fetched twice from the DB here and here

This issue occurs only in the cache-miss scenario. The first reference is used for ACTIONS_SPAN, while the second is specific to PAGES_SPAN. Aligning both to use the same page mono would be a minor adjustment

@sneha122 sneha122 added Task A simple Todo Integrations Product Issues related to a specific integration Integrations Pod General Issues related to the Integrations Pod that don't fit into other tags. labels Sep 11, 2024
@github-actions github-actions bot added Query & JS Pod Issues related to the query & JS Pod and removed Integrations Product Issues related to a specific integration labels Sep 11, 2024
@sneha122 sneha122 self-assigned this Sep 11, 2024
sneha122 added a commit that referenced this issue Sep 11, 2024
## Description
After perf updates made in PR
https://github.com/appsmithorg/appsmith/pull/36118/files, Page data DB
fetch call was getting triggered twice, one for PAGES_SPAN and one for
ACTIONS_SPAN. With this PR, we have replaced that a single Mono which is
being cached.

More details:
https://theappsmith.slack.com/archives/C024GUDM0LT/p1725960912325389


Fixes #`Issue Number`  
_or_  
Fixes #36243
> [!WARNING]  
> _If no issue exists, please create an issue first, and check with the
maintainers if the issue is valid._

## Automation

/ok-to-test tags="@tag.Sanity"

### 🔍 Cypress test results
<!-- This is an auto-generated comment: Cypress test results  -->
> [!TIP]
> 🟢 🟢 🟢 All cypress tests have passed! 🎉 🎉 🎉
> Workflow run:
<https://github.com/appsmithorg/appsmith/actions/runs/10808901838>
> Commit: d36df4c
> <a
href="https://internal.appsmith.com/app/cypress-dashboard/rundetails-65890b3c81d7400d08fa9ee5?branch=master&workflowId=10808901838&attempt=1"
target="_blank">Cypress dashboard</a>.
> Tags: `@tag.Sanity`
> Spec:
> <hr>Wed, 11 Sep 2024 09:49:39 UTC
<!-- end of auto-generated comment: Cypress test results  -->


## Communication
Should the DevRel and Marketing teams inform users about this change?
- [ ] Yes
- [x] No


<!-- This is an auto-generated comment: release notes by coderabbit.ai
-->
## Summary by CodeRabbit


- **Performance Improvements**
- Enhanced the page loading process by implementing a caching mechanism
for retrieving branched page data, potentially reducing database calls
and improving overall application performance.
- Streamlined the retrieval logic for branched pages, ensuring
consistent execution regardless of the base page ID state.

<!-- end of auto-generated comment: release notes by coderabbit.ai -->

---------

Co-authored-by: “sneha122” <“[email protected]”>
Shivam-z pushed a commit to Shivam-z/appsmith that referenced this issue Sep 26, 2024
## Description
After perf updates made in PR
https://github.com/appsmithorg/appsmith/pull/36118/files, Page data DB
fetch call was getting triggered twice, one for PAGES_SPAN and one for
ACTIONS_SPAN. With this PR, we have replaced that a single Mono which is
being cached.

More details:
https://theappsmith.slack.com/archives/C024GUDM0LT/p1725960912325389


Fixes #`Issue Number`  
_or_  
Fixes appsmithorg#36243
> [!WARNING]  
> _If no issue exists, please create an issue first, and check with the
maintainers if the issue is valid._

## Automation

/ok-to-test tags="@tag.Sanity"

### 🔍 Cypress test results
<!-- This is an auto-generated comment: Cypress test results  -->
> [!TIP]
> 🟢 🟢 🟢 All cypress tests have passed! 🎉 🎉 🎉
> Workflow run:
<https://github.com/appsmithorg/appsmith/actions/runs/10808901838>
> Commit: d36df4c
> <a
href="https://internal.appsmith.com/app/cypress-dashboard/rundetails-65890b3c81d7400d08fa9ee5?branch=master&workflowId=10808901838&attempt=1"
target="_blank">Cypress dashboard</a>.
> Tags: `@tag.Sanity`
> Spec:
> <hr>Wed, 11 Sep 2024 09:49:39 UTC
<!-- end of auto-generated comment: Cypress test results  -->


## Communication
Should the DevRel and Marketing teams inform users about this change?
- [ ] Yes
- [x] No


<!-- This is an auto-generated comment: release notes by coderabbit.ai
-->
## Summary by CodeRabbit


- **Performance Improvements**
- Enhanced the page loading process by implementing a caching mechanism
for retrieving branched page data, potentially reducing database calls
and improving overall application performance.
- Streamlined the retrieval logic for branched pages, ensuring
consistent execution regardless of the base page ID state.

<!-- end of auto-generated comment: release notes by coderabbit.ai -->

---------

Co-authored-by: “sneha122” <“[email protected]”>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Integrations Pod General Issues related to the Integrations Pod that don't fit into other tags. Query & JS Pod Issues related to the query & JS Pod Task A simple Todo
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant