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

example of loadable-cache can't run on my program. #197

Open
imalreadytaken opened this issue Feb 22, 2023 · 1 comment
Open

example of loadable-cache can't run on my program. #197

imalreadytaken opened this issue Feb 22, 2023 · 1 comment

Comments

@imalreadytaken
Copy link

imalreadytaken commented Feb 22, 2023

I tried the example of loadable-cache : https://github.com/eko/gocache#a-loadable-cache. (by the way, there is a mistake, the type of Book's ID must be int32).

After I called cacheManager.Get(context.Background(), "key") and Slept some time (I found the cache logic is running asynchronously), I tried run 'get key' in my redis-cli, but it returned -1.

By debugging, I found that there was an error when RedisStore Set the value:

redis: can't marshal *main.Book (implement encoding.BinaryMarshaler)

Is there any way I can make the example run successfully without implement BinaryMarshaler?

I tied the marshaler too, but there was a compile error:

Cannot use 'cacheLoadable' (type *LoadableCache[T]) as the type cache.CacheInterface[any] Type does not implement 'cache.CacheInterface[any]' need the method: Get(ctx context.Context, key any) (any, error) have the method: Get(ctx context.Context, key any) (T, error)

My golang version is go1.19.3 darwin/arm64.

@jasonlimantoro
Copy link

jasonlimantoro commented Aug 15, 2023

Experienced this as well.

I think there's a few issues with the loadable cache example:

// Example in the readme
type Book struct {
	ID string
	Name string
}

// Initialize Redis client and store
redisClient := redis.NewClient(&redis.Options{Addr: "127.0.0.1:6379"})
redisStore := redis_store.NewRedis(redisClient)

// Initialize a load function that loads your data from a custom source
loadFunction := func(ctx context.Context, key any) (*Book, error) {
    // ... retrieve value from available source
    return &Book{ID: 1, Name: "My test amazing book"}, nil
}

// Initialize loadable cache
cacheManager := cache.NewLoadable[*Book](
	loadFunction,
	cache.New[*Book](redisStore),
)

// ... Then, you can get your data and your function will automatically put them in cache(s)
  1. The part where Loadable cache asynchronously writes the cache doesn't have proper error handling
func (c *LoadableCache[T]) setter() {
	defer c.setterWg.Done()

	for item := range c.setChannel {
		c.Set(context.Background(), item.key, item.value) // this could return an error, should be logged
	}
}

In this case the error came from the redis client library's set method, which enforces the BinaryMarshaler implementation, which is why the data was not written to Redis in the first place. And since the error is not logged, it's hard to trace this issue.

Workaround for this is to implement the MarshalBinary:

func (b Book) MarshalBinary() (data []byte, err error) {
	return json.Marshal(b)
}
  1. After implementing the BinaryMarshaler, data is successfully written to the Redis but cacheManager.Get won't get the data from redis (nil) with no error.

This is because the T and value has a mismatched type.

  • T has *Book type (provided in the instantiation)
  • value has string type (result from redisClient.Get)

Therefore, the function would return a zero-ed T with nil error at the bottom.

// Get returns the object stored in cache if it exists
func (c *Cache[T]) Get(ctx context.Context, key any) (T, error) { // T here is provided with *Book type
	cacheKey := c.getCacheKey(key)

	value, err := c.codec.Get(ctx, cacheKey) // value here is a string
	if err != nil {
		return *new(T), err
	}

	if v, ok := value.(T); ok { // this would be false since value is a string
		return v, nil
	}

	return *new(T), nil // this will get returned
}

So one workaround for the second issue is to use string instead of *Book in the NewLoadable and Cache instantiations as well as return a the appropriate type for the loadFunction

loadFunction := func(ctx context.Context, key any) (string, error) {
	// ... retrieve value from available source
	b := &Book{ID: 1, Name: "My test amazing book"}
	bb, err := b.MarshalBinary() // this is the custom implementation required by redis client
	return string(bb), err
}

// Initialize loadable cache
cacheManager := cache.NewLoadable[string](
	loadFunction,
	cache.New[string](redisStore),
)

// cacheManager.Get() will work

I think this is nowhere ideal, since the loadFunction ideally should just return (*Book, error) and shouldn't care about marshaling / unmarshaling it.

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

No branches or pull requests

2 participants