Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

support LRU and LFU eviction #167

Closed
wants to merge 9 commits into from
Closed

support LRU and LFU eviction #167

wants to merge 9 commits into from

Conversation

suger-no
Copy link
Contributor

因为config中没有内存设置,所以逐出方法没有使用


//RandomDistinctKeysForEviction random get keys for eviction
func (db *DB) RandomDistinctKeysForEviction(limit int) []string {
if db.evictionPolicy.IsAllKeys() == false {
Copy link
Owner

Choose a reason for hiding this comment

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

通常写作 if !db.evictionPolicy.IsAllKeys()

if db.evictionPolicy.IsAllKeys() == false {
keys := make([]string, limit)
i := 0
db.data.ForEach(func(key string, val interface{}) bool {
Copy link
Owner

Choose a reason for hiding this comment

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

ttlMap 也有RandomDistinctKeys方法的

}

//RandomDistinctKeysForEviction random get keys for eviction
func (db *DB) RandomDistinctKeysForEviction(limit int) []string {
Copy link
Owner

Choose a reason for hiding this comment

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

这段代码只会调用一次,不需要抽象成函数

eviction/lfu.go Outdated
return key
}

func GetLFUCounter(lfu int32) uint8 {
Copy link
Owner

Choose a reason for hiding this comment

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

此函数不需要公开,应为私有

eviction/lfu.go Outdated
}

// LFULogIncr counter increase
func LFULogIncr(counter uint8) uint8 {
Copy link
Owner

Choose a reason for hiding this comment

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

此函数应私有

eviction/lfu.go Outdated
}

//LFUDecrAndReturn counter decr
func LFUDecrAndReturn(lfu int32) int32 {
Copy link
Owner

Choose a reason for hiding this comment

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

此函数应私有

eviction/lfu.go Outdated
}

// LFUTimeElapsed Obtaining the time difference from the last time
func LFUTimeElapsed(ldt int32) int32 {
Copy link
Owner

Choose a reason for hiding this comment

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

此函数应私有

Copy link
Owner

Choose a reason for hiding this comment

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

interface/eviction, eviction/lru, eviction/lfu 放入 database 包内即可,不需要公开到整个项目。
以及文件名拼错了 enviction_policy.go

}

//MakeEviction make a new mark about a key
func (db *DB) MakeEviction(keys []string) {
Copy link
Owner

Choose a reason for hiding this comment

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

MakeEviction 函数应私有,作为 MakeXXX 却无返回值就很别扭。可以改为 initEvitionMark

}

//UpdateMark update mark about eviction
func (db *DB) UpdateMark(keys []string) {
Copy link
Owner

Choose a reason for hiding this comment

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

  1. 函数应私有
  2. 这是 DB 的函数不是 evictionPolicy 的函数,函数名中带上 eviction 否则不知道更新的是什么 Mark.

UpdateMark -> updateEvictionMark

@HDT3213
Copy link
Owner

HDT3213 commented Apr 29, 2023

  1. bug fix:RewriteAof will panic if appendonly property is no 和 add Command command 两个提交已经合并了不要重复 PR。
  2. 请为新功能添加自动测试
  3. eviction 作为 database 的一个功能放入 database 包即可,无需添加到 interface 包。 可以添加 database/eviction 包,并在其中放入 interface.go,lru.go,lfu.go
  4. 请注意函数的可见范围

@HDT3213
Copy link
Owner

HDT3213 commented Apr 29, 2023

可以使用 process.NewProcess(int32(os.Getpid())) 获取自身进程的内存使用量,以此作为判断是否逐出的根据。

@suger-no
Copy link
Contributor Author

可以使用 process.NewProcess(int32(os.Getpid())) 获取自身进程的内存使用量,以此作为判断是否逐出的根据。

有几个问题,
如果拿到一个key要逐出,那么每次都要调用一次gc,关于gc的问题
还有逐出过程中需要加锁,性能瓶颈会在这产生?
process没发现这个包

@HDT3213
Copy link
Owner

HDT3213 commented May 5, 2023

evitction 应该写入 aof 并传播到 slave

@HDT3213
Copy link
Owner

HDT3213 commented May 5, 2023

redis 在达到 maxmemory 时不会只淘汰一个 key, 参考 getMaxmemoryState 计算 mem_tofree

@HDT3213
Copy link
Owner

HDT3213 commented May 5, 2023

请阅读 #146

@suger-no suger-no closed this by deleting the head repository May 17, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants