feat: added LRU cache for mockDB connections

[NilanshBansal]

Description

This PR implements an LRU (Least Recently Used) caching mechanism for the Mongo mockDB connections which get auto-cleaned up every 2 hours based on their access time.
This is done to stop the overpopulation of the open dangling connections to the mockDB resulting in the max connection limit.
The Caching Implementation used is Google Guave Cache.
Also refer - Working of Guava cache

Fixes #36474

Automation

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

🔍 Cypress test results

Tip

🟢 🟢 🟢 All cypress tests have passed! 🎉 🎉 🎉
Workflow run: https://github.com/appsmithorg/appsmith/actions/runs/11066462645
Commit: 38fcf57
Cypress dashboard.
Tags: @tag.Sanity
Spec:

---

Fri, 27 Sep 2024 08:01:28 UTC

Communication

Should the DevRel and Marketing teams inform users about this change?

• Yes
• No

Summary by CodeRabbit

• New Features

  • Introduced a new constant for the MongoDB plugin to enhance plugin identification.
  • Added a DatasourcePluginContext class to encapsulate datasource plugin context, including connection details and creation time.
  • Implemented a caching mechanism for datasource contexts to optimize connection management and reduce excessive database connections.
  • Added functionality to identify mock MongoDB datasources.

• Bug Fixes

  • Enhanced cleanup processes for stale connections in the caching system.

[coderabbitai]

Walkthrough

The changes introduced in this pull request include the addition of a new constant for the MongoDB plugin, the creation of a DatasourcePluginContext class to encapsulate datasource plugin details, and the implementation of a caching mechanism for managing connections to mock MongoDB. This caching strategy employs an LRU (Least Recently Used) approach to efficiently handle connections, ensuring that stale connections are closed and resources are managed effectively.

Changes

Files	Change Summary
app/server/appsmith-interfaces/src/main/java/com/appsmith/external/constants/PluginConstants.java	Added a new constant MONGO_PLUGIN for the MongoDB plugin.
app/server/appsmith-server/src/main/java/com/appsmith/server/domains/DatasourcePluginContext.java	Introduced DatasourcePluginContext class to hold connection details, plugin ID, and creation time.
app/server/appsmith-server/src/main/java/com/appsmith/server/services/ce/DatasourceContextServiceCEImpl.java	Implemented LRU caching for DatasourcePluginContext, added methods for handling mock MongoDB datasources and connection cleanup.

Assessment against linked issues

Objective	Addressed	Explanation
Implement LRU caching to cache and evict mock mongoDB connections (Issue #36474)	✅
Close stale connections to mockDB (Issue #36474)	✅

Poem

In the realm of code, connections do flow,
With Mongo's charm, we now have a glow.
Caching the mock, like a wise old sage,
Stale connections closed, we turn the page.
Cheers to the changes, let the plugins play,
In the world of Appsmith, we pave the way! 🎉

---

Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media?

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

🪧 Tips

Chat

There are 3 ways to chat with CodeRabbit:

• Review comments: Directly reply to a review comment made by CodeRabbit. Example:
  • I pushed a fix in commit <commit_id>, please review it.
  • Generate unit testing code for this file.
  • Open a follow-up GitHub issue for this discussion.
• Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
  • @coderabbitai generate unit testing code for this file.
  • @coderabbitai modularize this function.
• PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
  • @coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
  • @coderabbitai read src/utils.ts and generate unit testing code.
  • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.
  • @coderabbitai help me debug CodeRabbit configuration file.

Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments.

CodeRabbit Commands (Invoked using PR comments)

• @coderabbitai pause to pause the reviews on a PR.
• @coderabbitai resume to resume the paused reviews.
• @coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
• @coderabbitai full review to do a full review from scratch and review all the files again.
• @coderabbitai summary to regenerate the summary of the PR.
• @coderabbitai resolve resolve all the CodeRabbit review comments.
• @coderabbitai configuration to show the current CodeRabbit configuration for the repository.
• @coderabbitai help to get help.

Other keywords and placeholders

• Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
• Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
• Add @coderabbitai anywhere in the PR title to generate the title automatically.

Documentation and Community

• Visit our Documentation for detailed information on how to use CodeRabbit.
• Join our Discord Community to get help, request features, and share feedback.
• Follow us on X/Twitter for updates and announcements.

[coderabbitai]

Actionable comments posted: 4

Outside diff range and nitpick comments (5)

app/server/appsmith-server/src/main/java/com/appsmith/server/domains/DatasourcePluginContext.java (2)

9-12: Excellent use of Lombok annotations, students! But let's consider one more.

Your use of @getter, @Setter, and @tostring annotations is commendable. It keeps our code clean and readable. However, have you considered adding the @EqualsAndHashCode annotation as well? This would provide a proper implementation of the equals and hashCode methods, which could be useful if you need to compare DatasourcePluginContext objects or use them in collections.

Consider adding the @EqualsAndHashCode annotation:

 @Getter
 @Setter
 @ToString
+@EqualsAndHashCode
 public class DatasourcePluginContext<T> {

---

1-20: Overall, a commendable effort in supporting our caching strategy!

Class, your DatasourcePluginContext is a solid foundation for our LRU caching mechanism. It aligns well with our objective of managing mockDB connections more effectively. The inclusion of a creationTime field is particularly useful for implementing cache eviction based on age.

To further improve:

1. Consider adding documentation comments to explain the purpose of this class and its fields.
2. You might want to implement a getAge() method that calculates the time elapsed since creation, which could be useful for the LRU algorithm.

Remember, clear and well-structured code like this is key to solving our connection management issues (problem #36474). Keep up the good work!

app/server/appsmith-interfaces/src/main/java/com/appsmith/external/constants/PluginConstants.java (1)

27-27: Class, pay attention to this new addition!

The addition of the MONGO_PLUGIN constant is a welcome improvement to our constants collection. It's like adding a new word to our vocabulary! However, just as we arrange words alphabetically in a dictionary, it would be beneficial to maintain alphabetical order in our constants list.

Consider reordering the constants alphabetically within the PackageName interface. This will make it easier for your fellow students to find specific constants in the future. Remember, organization is key to maintaining a tidy codebase!

app/server/appsmith-server/src/main/java/com/appsmith/server/services/ce/DatasourceContextServiceCEImpl.java (2)

53-53: Please pay attention to spacing in your comments.

In line 53, there's a missing space after the period. Proper spacing enhances readability. It should read: "connection. The cleanup process is performed after every 2 hours."

---

54-54: Clarify the term "movies mockDB" for better understanding.

Using specific references like "movies mockDB" might be confusing if it's not commonly known. Consider using "the mockDB" or a more general term to make your documentation clear to all readers.

Review details

Configuration used: .coderabbit.yaml
Review profile: CHILL

Commits

Files that changed from the base of the PR and between 8fe96c9 and 4fdbab1.

Files selected for processing (3)

• app/server/appsmith-interfaces/src/main/java/com/appsmith/external/constants/PluginConstants.java (1 hunks)
• app/server/appsmith-server/src/main/java/com/appsmith/server/domains/DatasourcePluginContext.java (1 hunks)
• app/server/appsmith-server/src/main/java/com/appsmith/server/services/ce/DatasourceContextServiceCEImpl.java (9 hunks)

Additional comments not posted (5)

app/server/appsmith-server/src/main/java/com/appsmith/server/domains/DatasourcePluginContext.java (1)

1-8: Well done, class! The package declaration and imports are spot on.

You've correctly declared the package and imported the necessary Lombok annotations and the Instant class. This shows good organization and understanding of the tools at your disposal.

app/server/appsmith-server/src/main/java/com/appsmith/server/services/ce/DatasourceContextServiceCEImpl.java (4)

220-221: Excellent use of cache access for updating last accessed time.

Accessing the cache with getIfPresent is a good strategy to update the entry's last accessed time without affecting its value. This helps maintain the entry in the cache appropriately.

---

246-269: Good implementation of conditional caching for mock Mongo datasources.

Your logic correctly ensures that only mock Mongo datasources are cached in the LRU cache. This is an effective way to manage resources specific to mock datasources.

---

297-297: Verify the parameters in updateDatasourceStorage call.

It's important to confirm that passing FALSE and false as parameters to updateDatasourceStorage aligns with the method's expectations. Please ensure these parameters correctly represent the desired behavior.

---

503-507: Proper cache invalidation upon datasource context deletion.

Invalidating the LRU cache when a datasource context is deleted helps prevent stale data and is a good practice.

[subrata71]

I overlooked this point during my review. This is an in-memory cache, not shared across different pods. Doesn’t that mean the cache will only be effective for subsequent requests to the same pod? Each pod will have its own cache, which could lead to excessive caching and undermine the original goal.

[NilanshBansal]

I overlooked this point during my review. This is an in-memory cache, not shared across different pods. Doesn’t that mean the cache will only be effective for subsequent requests to the same pod? Each pod will have its own cache, which could lead to excessive caching and undermine the original goal.

@subrata71 yes the cache is not shared across the different pods and as per the current and projected traffic on mockDB connections we are reaching the limit of 3k connections in 72 hours combined on all pods.
The eviction time added in the above configuration is 2 hours per pod which can only lead to excessive caching only when there are 3k connections in less than 2 hours combined on all pods which is not the case that we are projecting.
Saying that, do you still find the need to add a cache size limit @subrata71 ?

[subrata71]

@NilanshBansal
Could you please add more logging to track when the cache expires? Including information like the cache size would help with monitoring across pods and allow for better adjustments to the caching implementation.

[NilanshBansal]

As per discussion with @subrata71, I have added the maximum size limit of the cache per pod and also added the logs around the cache size after addition and eviction.
@subrata71 can you please re-review 🙏

[NilanshBansal]

Tag.All has already passed on this PR hence updated with Tag.Sanity as only log level changes are done post it.

[github-actions]

Failed server tests

• com.appsmith.server.solutions.CreateDBTablePageSolutionTests#createPageWithValidPageIdForGoogleSheet

[subrata71]

LGTM