Skip to content

Add redis key prefix as an option to Redis cluster - #975

Merged
feast-ci-bot merged 1 commit into
feast-dev:masterfrom
khorshuheng:redis-prefix
Sep 7, 2020
Merged

Add redis key prefix as an option to Redis cluster#975
feast-ci-bot merged 1 commit into
feast-dev:masterfrom
khorshuheng:redis-prefix

Conversation

@khorshuheng

@khorshuheng khorshuheng commented Sep 1, 2020

Copy link
Copy Markdown
Collaborator

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?:

Additional optional configuration for Redis cluster, where user can specified a prefix to be prepended to the Redis keys written / retrieved.

@khorshuheng khorshuheng added kind/feature New feature or request and removed size/M labels Sep 1, 2020
@khorshuheng khorshuheng changed the title Add redis key prefix as an option to Redis cluster (WIP) Add redis key prefix as an option to Redis cluster Sep 1, 2020
@khorshuheng khorshuheng changed the title (WIP) Add redis key prefix as an option to Redis cluster Add redis key prefix as an option to Redis cluster Sep 1, 2020
@pyalex

pyalex commented Sep 2, 2020

Copy link
Copy Markdown
Collaborator

Since we may change prefix during project lifetime (with new release, for example), falling mechanism is needed to use both new & old prefixes.

@khorshuheng

Copy link
Copy Markdown
Collaborator Author

/test test-end-to-end

@khorshuheng

Copy link
Copy Markdown
Collaborator Author

/retest test-end-to-end

@khorshuheng

Copy link
Copy Markdown
Collaborator Author

/retest

@terryyylim

Copy link
Copy Markdown
Member

/retest

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

do we need to mock serializer? why not to use actual implementation?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

I have replaced the mock serializer with actual implementation.

@khorshuheng

Copy link
Copy Markdown
Collaborator Author

/test test-end-to-end-batch

@khorshuheng

Copy link
Copy Markdown
Collaborator Author

/test test-end-to-end

@feast-ci-bot

Copy link
Copy Markdown
Collaborator

[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

Details Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@pyalex

pyalex commented Sep 7, 2020

Copy link
Copy Markdown
Collaborator

/lgtm

@feast-ci-bot
feast-ci-bot merged commit ff43765 into feast-dev:master Sep 7, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants