Skip to content

Conversation

@denis-chernov-smartcontract

Closes #DS-1422

Description

Initial commit with cache implementation, with required types, and helper functions.

@changeset-bot
Copy link

changeset-bot bot commented Nov 24, 2025

⚠️ No Changeset found

Latest commit: 5bd3504

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

Copy link

@ro-tex ro-tex left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good so far! I like the method docstrings.
We'll need some tests at some point but it can be further down the line.

Comment on lines +98 to +99
// Items returns all items in the cache
func (c *Cache) Items() map[string]*types.CacheItem {
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure what the use case for this method is.

Are we OK with returning pointers to the actual data? OTOH, if we don't return pointers we might run OOM

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The use case for this is a debug API endpoint (/debug/cache/items) that returns the entire cache.
@brunotm need your opinion on pointers.

@mxiao-cll
Copy link
Contributor

I am not sure if this is the correct repo

@denis-chernov-smartcontract
Copy link
Author

I am not sure if this is the correct repo

This is the correct repo.
We are adding a Go service to handle the high load generated by Data Streams.
@mxiao-cll initially, we wanted to create a separate adapters implementation for Data Streams, but received some pushback in favor of this solution.

Design document: https://docs.google.com/document/d/18opUJqvmrgQjy5UHjmEnwv22zdSJOre4o7-wnk3trqs/edit?tab=t.0#heading=h.b8hxx51vbzqg
@alejoberardino and @brunotm can provide more details.

@alejoberardino
Copy link
Collaborator

I am not sure if this is the correct repo

This is the correct repo. We are adding a Go service to handle the high load generated by Data Streams. @mxiao-cll initially, we wanted to create a separate adapters implementation for Data Streams, but received some pushback in favor of this solution.

Design document: https://docs.google.com/document/d/18opUJqvmrgQjy5UHjmEnwv22zdSJOre4o7-wnk3trqs/edit?tab=t.0#heading=h.b8hxx51vbzqg @alejoberardino and @brunotm can provide more details.

Happy to chat about this in DMs, as far as repo placement this seems like an easy place but I could also see it hosted somewhere else entirely, don't think we went that deep in our high level solution discussions

@mxiao-cll
Copy link
Contributor

I am not sure if this is the correct repo

This is the correct repo. We are adding a Go service to handle the high load generated by Data Streams. @mxiao-cll initially, we wanted to create a separate adapters implementation for Data Streams, but received some pushback in favor of this solution.
Design document: docs.google.com/document/d/18opUJqvmrgQjy5UHjmEnwv22zdSJOre4o7-wnk3trqs/edit?tab=t.0#heading=h.b8hxx51vbzqg @alejoberardino and @brunotm can provide more details.

Happy to chat about this in DMs, as far as repo placement this seems like an easy place but I could also see it hosted somewhere else entirely, don't think we went that deep in our high level solution discussions

I think prob better to be in a brand new repo?

Copy link

@ro-tex ro-tex left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

Comment on lines +76 to +78
if err != nil {
return nil
}
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am not sure how helpful logging the error would be but I wanted to flag it as an idea. Right now this will error out and we won't know why.

@denis-chernov-smartcontract denis-chernov-smartcontract merged commit 27fef12 into streams-adapters Dec 4, 2025
@denis-chernov-smartcontract denis-chernov-smartcontract deleted the DS-1422/cache-implementation branch December 4, 2025 17:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants