eko/gocache

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

imalreadytaken opened this issue · 1 comments

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.

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.