-
Notifications
You must be signed in to change notification settings - Fork 34
Replace Redis with Apache Ignite for in memory cache and db #129
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is a very good start. Please take a look at the comments. Thanks!
@@ -9,3 +9,7 @@ logging.level.root=info | |||
logging.level.org.springframework.web=info | |||
logging.file.path=. | |||
logging.type=file | |||
#Ignite configuration | |||
#ignite.host=localhost |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is a very good start with the abstraction layer on DB access.
Couple of suggestions:
- We likely want to uncomment the ignite configuration.
- Place ignite cluster on other hosts, and support remote IPs/URLs instead of localhost
- Deploy Ignite clusters as a K8s service
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think the remaining item is to update the one-box deployment script as well as the k8s yaml file, respectively.
https://github.com/futurewei-cloud/alcor/blob/master/scripts/deploy.sh
https://github.com/futurewei-cloud/alcor/tree/master/kubernetes
How did you test the change locally? deploy a Ignite container and verify locally?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let us address this item in next PR.
|
||
public class IgniteCache<K, V> implements ICache<K, V> { | ||
private static final Logger logger = LoggerFactory.getLogger(); | ||
private static final int QUERY_PAGE_SIZE = 1000; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do we know how the page size impacts perf? Could we make it configurable?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The static var is gone now. Not sure if we need it.
src/com/futurewei/alcor/controller/cache/repo/ICacheRepository.java
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This iteration looks a lot better. Thank you.
Can you address the remaining comments and update the title and description of this PR, before you merge the change?
Merge the PR to master. Deployment step to be completed in a separate PR. |
This PR implements the abstraction layer of the in-memory database, which i also call it db cache. underlying component that really provide cache services that include redis and ignite currently. The ICache interface defines the basic interface for cache operations, such as add, delete, and modify. The cache also supports Transaction,Transaction interface provides transaction-related interfaces. If the underlying cache chooses ignite, it also supports security features and uses SLL to establish a secure link with the ignite server.