Add redis key prefix as an option to Redis cluster #975
Conversation
khorshuheng
requested review from
davidheryanto,
pyalex,
woop and
zhilingc
as code owners
khorshuheng
changed the title
khorshuheng
changed the title
|
Since we may change prefix during project lifetime (with new release, for example), falling mechanism is needed to use both new & old prefixes. |
khorshuheng
force-pushed
the
redis-prefix
branch
from
5c65bd8
to
dd04ec5
Compare
khorshuheng
force-pushed
the
redis-prefix
branch
from
dd04ec5
to
c69e353
Compare
|
/test test-end-to-end |
|
/retest test-end-to-end |
|
/retest |
khorshuheng
force-pushed
the
redis-prefix
branch
from
c69e353
to
e0508d0
Compare
|
/retest |
| Field.newBuilder().setValue(intValue(1)).build(), | ||
| Field.newBuilder().setValue(intValue(1)).build())) | ||
| .build()); | ||
| when(serializer.serialize(redisKeys.get(0))).thenReturn(serializedKey1); |
There was a problem hiding this comment.
do we need to mock serializer? why not to use actual implementation?
There was a problem hiding this comment.
I have replaced the mock serializer with actual implementation.
khorshuheng
force-pushed
the
redis-prefix
branch
from
e0508d0
to
05a2306
Compare
|
/test test-end-to-end-batch |
|
/test test-end-to-end |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: khorshuheng, pyalex The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
|
/lgtm |
What this PR does / why we need it:
There are scenarios where multiple Feast deployment are sharing the same Redis cluster, but key collision between the different deployment should be avoided. Since Redis cluster doesn't support database, attaching prefix to keys is a good workaround to achive namespacing.
To facilitate migration of redis keys from one prefix to another, fallback mechanism has also been introduced. If enabled, when a key is not found in the database, a fallback prefix can be used instead. However, this will results in more calls to Redis, and additional latency.
Which issue(s) this PR fixes:
Fixes #
Does this PR introduce a user-facing change?: