feat(redis): implement redis caches #1
Conversation
implement cache interfaces based on the redis cache
| type digest struct { | ||
| Digest mh.Multihash | ||
| } | ||
| type digest mh.Multihash | ||
|
|
||
| func (d digest) hasMultihash() {} |
There was a problem hiding this comment.
ha yea actually was just messing around but it's simpler.
| } | ||
|
|
||
| // RequireEqualIndex compares two sharded dag indexes to verify their equality | ||
| func RequireEqualIndex(t *testing.T, expectedIndex blobindex.ShardedDagIndexView, actualIndex blobindex.ShardedDagIndexView) { |
There was a problem hiding this comment.
Would it be better to blobindex.Archive(...) these and compare the bytes? If we ever add properties to these objects this function could return a false positive...
That said, this is more useful info when they're not equal.
There was a problem hiding this comment.
yea this is actually used to check a archive/unarchive operation so it seems redundant at this point.
| } | ||
|
|
||
| // RequireEqualDelegation compares two delegations to verify their equality | ||
| func RequireEqualDelegation(t *testing.T, expectedDelegation delegation.Delegation, actualDelegation delegation.Delegation) { |
There was a problem hiding this comment.
Would it be better to delegation.Archive(...) these and compare the bytes? See previous comment for reasoning.
There was a problem hiding this comment.
yea again, I'm testing archive/unarchive so this seems better signal
pkg/metadata/metadata.ipldsch
Outdated
| @@ -0,0 +1,5 @@ | |||
| type ContentClaimMetadata struct { | |||
| ClaimType Int | |||
| ShortCut Bytes | |||
There was a problem hiding this comment.
🙏 can this be named better? Something like "embedded data"/"cached data"/"claimed data"?
| return delegation.Extract([]byte(data)) | ||
| } | ||
|
|
||
| func delegationToRedis(d delegation.Delegation) (string, error) { |
There was a problem hiding this comment.
You can store bytes in redis no?
There was a problem hiding this comment.
yes, but everything is a string to anyway.
pkg/redis/providerresult.ipldsch
Outdated
| type PeerID bytes | ||
| type Multiaddr bytes |
There was a problem hiding this comment.
| type PeerID bytes | |
| type Multiaddr bytes | |
| type PeerID Bytes | |
| type Multiaddr Bytes |
There was a problem hiding this comment.
that actually didn't work. I think when you define alias types you have to use the lower case.
| } | ||
|
|
||
| // Set saves a serialized value to redis | ||
| func (rs *Store[Key, Value]) Set(ctx context.Context, key Key, value Value, expires bool) error { |
There was a problem hiding this comment.
What's the reason to use a boolean vs a duration for expiration?
There was a problem hiding this comment.
I dunno simplicity given it's an abstract interface. also the key distinction I'm thinking of is keys you can remove to keep the cache size down vs keys you dont want to do that on.
| } | ||
|
|
||
| // SetExpirable changes the expiration property for a given key | ||
| func (rs *Store[Key, Value]) SetExpirable(ctx context.Context, key Key, expires bool) error { |
There was a problem hiding this comment.
When do we want something that previously had no expiry to have one (and vice versa)?
There was a problem hiding this comment.
it's actually the other way around -- I cached on write before IPNI, so I set NO expiry. Now i want to set YES expiry cause I've confirmed it's actually on IPNI now.
| expires time.Duration | ||
| } | ||
|
|
||
| type MockRedis struct { |
There was a problem hiding this comment.
There's a mock redis implementation here https://github.com/go-redis/redismock - is this not suitable?
There was a problem hiding this comment.
yea, I saw this -- generally, I want something a bit more stateful, that's simple and which I understand. I often that particular style of mocking library (seems analogous to this https://github.com/uber-go/mock) quite frustrating and awkward to use, and ultimately it doesn't tell you much. By and large, I just write my own mocks in go, cause I want to understand exactly the behavior that's happening under the hood and to see what comes out of that.
There was a problem hiding this comment.
there's also this -- https://github.com/alicebob/miniredis -- basically the far other end running an in process redis
ultimately I looked at both and was like "it would be easiest for me to write tests against something I expected to behave a very specific way.
Co-authored-by: Alan Shaw <[email protected]>
|
@alanshaw can you clarify what your blocking changes requested are? I've responded on several of thees but I'm not sure whether it makes sense to change. |
also updates metadata design implements a parallel walk implements ProviderIndex
also adds various fixes, tweeks, and rearchitects to prepare for testing
feat: server scaffolding
adds job queue, serialization, other features
Co-authored-by: Alan Shaw <[email protected]>
Query Implementation
Goal
Implement the three types of cache interfaces based on the redis cache
Implementation