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

feat: add zset #98

Merged
merged 21 commits into from
Nov 3, 2021
Merged

feat: add zset #98

merged 21 commits into from
Nov 3, 2021

Conversation

SilverRainZ
Copy link
Member

@SilverRainZ SilverRainZ commented Oct 12, 2021

Intro

Package zset provides a concurrent-safety sorted set, can be used as a local replacement of Redis' zset (https://redis.com/ebook/part-2-core-concepts/chapter-3-commands-in-redis/3-5-sorted-sets/).

The main different to other sets is, every value of set is associated with a score, that is used in order to take the sorted set ordered, from the smallest to the greatest score.

The sorted set has O(log(N)) time complexity when doing Add(ZADD) and Remove(ZREM) operations and O(1) time complexity when doing Contains operations.

Package zset is a go implmentation of https://github.com/redis/redis/blob/unstable/src/t_zset.c

Comparison

Redis command Go function
ZADD Add
ZINCRBY IncrBy
ZREM Remove
ZREMRANGEBYSCORE RemoveRangeByScore
ZREMRANGEBYRANK RemoveRangeByRank
ZUNION Union
ZINTER Inter
ZINTERCARD TODO
ZDIFF TODO
ZRANGE Range
ZRANGEBYSCORE IncrBy
ZREVRANGEBYSCORE RevRangeByScore
ZCOUNT Count
ZREVRANGE RevRange
ZCARD Len
ZSCORE Score
ZRANK Rank
ZREVRANK RevRank
ZPOPMIN TODO
ZPOPMAX TODO
ZRANDMEMBER TODO

List of redis commands are generated from the following command:

cat redis/src/server.c | grep -o '"z.*",z.*Command' | grep -o '".*"' | cut -d '"' -f2

You may find that not all redis commands have corresponding go implementations,
the reason is as follows:

Unsupported Commands

Redis' zset can operates elements in lexicographic order, which is not commonly
used function, so zset does not support commands like ZREMRANGEBYLEX, ZLEXCOUNT
and so on.

Redis command
ZREMRANGEBYLEX
ZRANGEBYLEX
ZREVRANGEBYLEX
ZLEXCOUNT

In redis, user accesses zset via a string key. We do not need such string key
because we have variable. so the following commands are not implemented:

Redis command
ZUNIONSTORE
ZINTERSTORE
ZDIFFSTORE
ZRANGESTORE
ZMSCORE
ZSCAN

Tests

Unittests

coverage: 94.0% of statements

Benchmark

go test -v -cpuprofile cpu.pprof -cpu=1 -run=NOTEST -bench=. -benchmem -benchtime=100000x -count=10 -timeout=60m > y.txt && benchstat y.txt
name                                  time/op
Contains100Hits/sortedset             39.5ns ± 1%
Contains50Hits/sortedset              43.4ns ± 4%
ContainsNoHits/sortedset              21.9ns ± 5%
Add/sortedset                         53.6ns ± 1%
1Add99Contains/sortedset              38.1ns ±12%
10Add90Contains/sortedset             47.6ns ± 5%
50Add50Contains/sortedset             56.9ns ± 3%
1Add3Incr6Remove90Contains/sortedset  49.3ns ±11%

name                                  alloc/op
Contains100Hits/sortedset              0.00B
Contains50Hits/sortedset               0.00B
ContainsNoHits/sortedset               0.00B
Add/sortedset                          2.00B ± 0%
1Add99Contains/sortedset               0.00B
10Add90Contains/sortedset              0.00B
50Add50Contains/sortedset              1.00B ± 0%
1Add3Incr6Remove90Contains/sortedset  0.50B ±100%

name                                  allocs/op
Contains100Hits/sortedset               0.00
Contains50Hits/sortedset                0.00
ContainsNoHits/sortedset                0.00
Add/sortedset                           0.00
1Add99Contains/sortedset                0.00
10Add90Contains/sortedset               0.00
50Add50Contains/sortedset               0.00
1Add3Incr6Remove90Contains/sortedset    0.00

TODOs

  • implementation
  • godoc
  • unittest
  • benchmark
  • reamde

@SilverRainZ SilverRainZ marked this pull request as ready for review October 13, 2021 03:05
@luchsh
Copy link
Collaborator

luchsh commented Oct 13, 2021

/benchdiff


nothing happened; guess this issue should depend on #95

Copy link
Member

@zhangyunhao116 zhangyunhao116 left a comment

Choose a reason for hiding this comment

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

Maybe we could support NewInt in this pull request? (In next PR is also ok)

collection/zset/zset.go Outdated Show resolved Hide resolved
collection/zset/zset.go Outdated Show resolved Hide resolved
collection/zset/zset.go Outdated Show resolved Hide resolved
collection/zset/zset.go Outdated Show resolved Hide resolved
collection/zset/zset.go Outdated Show resolved Hide resolved
@SilverRainZ
Copy link
Member Author

Maybe we could support NewInt in this pull request? (In next PR is also ok)

I prefer in next PR, just a code generateion stuff, we focus on the core implementaion in this PR.

Copy link
Member

@zhangyunhao116 zhangyunhao116 left a comment

Choose a reason for hiding this comment

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

一些风格问题,统一一下就好啦,已经加了 suggestion

collection/zset/oparry.go Outdated Show resolved Hide resolved
collection/zset/zset.go Outdated Show resolved Hide resolved
collection/zset/zset.go Outdated Show resolved Hide resolved
collection/zset/zset.go Outdated Show resolved Hide resolved
collection/zset/zset.go Outdated Show resolved Hide resolved
collection/zset/zset.go Outdated Show resolved Hide resolved
collection/zset/zset.go Outdated Show resolved Hide resolved
collection/zset/zset.go Outdated Show resolved Hide resolved
collection/zset/zset.go Outdated Show resolved Hide resolved
collection/zset/zset.go Outdated Show resolved Hide resolved
@luchsh luchsh self-requested a review October 19, 2021 05:27
@SilverRainZ
Copy link
Member Author

Any progress?

@SilverRainZ SilverRainZ added the enhancement New feature or request label Oct 29, 2021
| ZREMRANGEBYLEX | NOT SUPPORTED |
| ZUNIONSTORE | UnionStore |
| ZINTERSTORE | InterStore |
| ZDIFFSTORE | |
Copy link
Collaborator

Choose a reason for hiding this comment

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

这些空着的command是什么状态?

Copy link
Member Author

Choose a reason for hiding this comment

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

在内部的需求文档里没覆盖到,因此这次先不实现,以后可能会实现。

Copy link
Collaborator

Choose a reason for hiding this comment

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

<NOT SUPPORTED>, <TODO>

Copy link
Member Author

Choose a reason for hiding this comment

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

Done, 用斜体区分了,<> 在 markdown 里不友好。

collection/zset/skiplist.go Show resolved Hide resolved

const (
maxLevel = 32 // same to ZSKIPLIST_MAXLEVEL
p = 0.25 // same to ZSKIPLIST_P
Copy link
Collaborator

Choose a reason for hiding this comment

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

作为常量名字稍微多些清晰的意义吧

Copy link
Member Author

Choose a reason for hiding this comment

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

这里是保持和 redis 的常量名一致,也许后面加点注释会好一点?

Copy link
Member Author

Choose a reason for hiding this comment

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

改成 probability 了。

collection/zset/oparry.go Show resolved Hide resolved
value: value,
level: level,
}
if level > op1 {
Copy link
Collaborator

Choose a reason for hiding this comment

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

如果写成 node.oparr.init(level) 会不会更模块化一些?

Copy link
Member Author

Choose a reason for hiding this comment

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

是的,之前想这么写,但处于保持一致的原因没有这么做…

Copy link
Member Author

Choose a reason for hiding this comment

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

done。

collection/zset/zset.go Show resolved Hide resolved
collection/zset/zset.go Show resolved Hide resolved
## Features

- Concurrent safe API
- Value is sorted with score
Copy link
Collaborator

Choose a reason for hiding this comment

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

Values are always sorted by their score ? 但这句话听起来像是实现上的做法, 那用户能感知的特点是啥呢?比如快速的range、rank等api?

Copy link
Member Author

Choose a reason for hiding this comment

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

我理解这就是元素按分数排序就是对 zset 的主要需求了?

实现上应该说用 skiplist blahblah……

Copy link
Member Author

Choose a reason for hiding this comment

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

只改了语法……

collection/zset/readme.md Outdated Show resolved Hide resolved
collection/zset/skiplist.go Show resolved Hide resolved
@luchsh
Copy link
Collaborator

luchsh commented Oct 29, 2021

Any progress?

pls @ the reviewers or shout at them in Feishu :)

| ZREMRANGEBYRANK | RemoveRangeByRank |
| ZREMRANGEBYLEX | *NOT SUPPORTED* |
| ZUNIONSTORE | *NOT SUITABLE* |
| ZINTERSTORE | *NOT SUITABLE* |
Copy link
Collaborator

Choose a reason for hiding this comment

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

unsuitable?

Copy link
Member Author

Choose a reason for hiding this comment

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

重新整理了格式,把不打算支持的命令分开放了。

luchsh
luchsh previously approved these changes Nov 3, 2021
collection/zset/readme.md Outdated Show resolved Hide resolved
collection/zset/readme.md Outdated Show resolved Hide resolved
zhangyunhao116
zhangyunhao116 previously approved these changes Nov 3, 2021
luchsh
luchsh previously approved these changes Nov 3, 2021
@SilverRainZ SilverRainZ merged commit d4719f7 into develop Nov 3, 2021
@SilverRainZ SilverRainZ deleted the feat/zset branch November 3, 2021 09:05
zhangyunhao116 added a commit that referenced this pull request Jan 18, 2022
* release: 20210913 (#83) (#88)

* feat: circuitbreaker.panel use skipmap (#78)

* fix(metainfo): fix misuse of append (#79)

* fix(lscq): add write barrier for LSCQ Pointer (#80)

* feat(metainfo): improve backward APIs (#81)

* release: 20210913 (#83) (#93)

* feat: circuitbreaker.panel use skipmap (#78)

* fix(metainfo): fix misuse of append (#79)

* fix(lscq): add write barrier for LSCQ Pointer (#80)

* feat(metainfo): improve backward APIs (#81)

* chore(skipmap,skipset): remove duplicated code generation declaration (#87)

* fix(lscq): cas2 use runtime.noescape (#94)

* feat: add fastrand.Read() (#90)

Co-authored-by: liyichao <liyichao@bytedance.com>

* doc: add badge to link to godoc (#99)

* ci: skip golang related workflows when there are only doc changes (#101)

* ci: add workflow for feishu/lark notification (#100)

* ci: add workflow for feishu/lark notification

* ci: fix typo

* ci: also send feishu notification on issue opened

Co-authored-by: Jonathan Lu <luchsh@users.noreply.github.com>

* feat(metainfo): define standard for backward prefix (#102)

* feat(fastrand): support Read,Shuffle,Perm (#103)

* feat(fastrand): Read remove temp buffer (#104)

* ci: add github workflow for performance regression check (#95)

* ci: add github workflow for performance regression check

Comment "/benchdiff" to trigger this check

* ci: add comments to pr-benchdiff.yml

* ci: report benchdiff result to PR_HEAD's check run

* ci: various changes

- support "pull_reqeust" event
- filter deleted go packages
- post a comment on job started
- post a comment on job failed

* ci: let xargs ignore empty line

* ci: skip benchdiff when there are only doc changes

* ci: set -x for debugging

* ci: make sure we are using github default branch as baseline

* ci: set benchtime=1s for benchdiff

* chore: add github issue forms (#105)

* test(lang/syncx): add benchmark for RWMutex (#89)

* fix(xxhash3): add fallback to fix panic occurs on non avx2, non sse2 machine (#108)

Co-authored-by: Ye Li <liye.e@bytedance.com>

* feat: add zset (#98)

* feat: add zset

* chore(zset): update comment

* docs(zset): add readme

* chore: some code style fixes

* chore(zset): rename Float64RangeOpt -> RangeOpt

* chore(zset): comment style fixes

* chore(zset): add a todo about maxLevel

* chore(zset): another comment fix

* chore(zset): move skiplist impl to another file

* chore(zset): add license header for opt.go

* chore: add myself as zset's CODEOWNER

* zset: don't use z.Range(0, z.Len()-1)

* doc: remove redundant section

* fix(zset): break when key is not exist

* chore(zset): simplify func name

* chore(zset): update cheatsheet

* chore(zset): meaningful const

* refactor(zset): optionalArray.init()

* docs: update zset readme

* docs(zset): also add @zhangyunhao116 as code owner

* docs(zset): some grammar fixes

* docs: fix docs typo (#109)

* feat: auto tuning gc (#112)

* feat: auto tuning gc

* doc: gctuner

* fix: gctuner tests data race (#113)

* fix: gctuner tests data race

* chore: add owner

* chore(ci): use self-hosted runner (#114)

* chore(ci): use self-hosted runner

* fix lint

Co-authored-by: Shengyu Zhang <reg@silverrainz.me>
Co-authored-by: ziposcar <499581494@qq.com>
Co-authored-by: liyichao <liyichao@bytedance.com>
Co-authored-by: Shengyu Zhang <zhangshengyu.0@bytedance.com>
Co-authored-by: Jonathan Lu <luchsh@users.noreply.github.com>
Co-authored-by: Pure White <wudi.daniel@bytedance.com>
Co-authored-by: lyeeeeee <awesomeyeli@gmail.com>
Co-authored-by: Ye Li <liye.e@bytedance.com>
Co-authored-by: chyroc <chyroc@qq.com>
Co-authored-by: Joway <joway.w@gmail.com>
Co-authored-by: Joway <wangzhuowei@bytedance.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants