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: implement sequencer API with postgresql #567

Closed
wants to merge 13 commits into from
Closed

feat: implement sequencer API with postgresql #567

wants to merge 13 commits into from

Conversation

azhsmesos
Copy link
Contributor

What this PR does:
新增基于postgresql实现分布式自增唯一id组件
Which issue(s) this PR fixes: 利用号段模式+双buffer,基于postgresql实现分布式自增id组件

Fixes #

Special notes for your reviewer:

Does this PR introduce a user-facing change?: 主要修改在components/sequencer/postgresql包下


@mosn-community-bot
Copy link

Hi @azhsmesos, welcome to mosn community, Please sign Contributor License Agreement!

After you signed CLA, we will automatically sync the status of this pull request in 3 minutes.

@azhsmesos
Copy link
Contributor Author

请问merge的审查什么时候呢, 过与不过会告知吗

@seeflood
Copy link
Member

seeflood commented May 14, 2022

感谢贡献!! 在跑CI啦,可以先修复一下 CI 报错
比如 PR的标题需要改成 feat: implement sequencer API with postgresql这种格式(用英文哦)
除了 CI 跑自动测试,还会有社区维护者来 code review,code review 如果有修改意见,会在这个 PR 下留言的哈
code review 有两票 approve 后, PR 就可以合并~

@azhsmesos azhsmesos changed the title 解决issue #554 feat: implement sequencer API with postgresql May 14, 2022
@azhsmesos azhsmesos changed the title feat: implement sequencer API with postgresql implement sequencer API with postgresql May 14, 2022
@azhsmesos
Copy link
Contributor Author

感谢贡献!! 在跑CI啦,可以先修复一下 CI 报错 比如 PR的标题需要改成 feat: implement sequencer API with postgresql这种格式(用英文哦) 除了 CI 跑自动测试,还会有社区维护者来 code review,code review 如果有修改意见,会在这个 PR 下留言的哈 code review 有两票 approve 后, PR 就可以合并~

这个cr我可以手动触发吗,每次我commit后就手动触发跑一下,第二就是咱们pr有没有一个规格文档来介绍,比如需要保证什么格式啥的,类名规格这些的

@seeflood
Copy link
Member

seeflood commented May 14, 2022

这个cr我可以手动触发吗,每次我commit后就手动触发跑一下

github 有个限制,每个仓库的 first-time contributor 提的pr ,不能自动跑CI,每次都得维护者手动点CI。合并一次pr、成为 contributor 后,就能自动触发 CI 啦。
为了方便开发, layotto 有一套 make 脚本,可以在本地跑检查、跑测试,你本地敲 make all即可,详见文档说明 https://mosn.io/layotto/#/zh/development/commands

@seeflood
Copy link
Member

第二就是咱们pr有没有一个规格文档来介绍,比如需要保证什么格式啥的,类名规格这些的

Layotto 贡献者指南Layotto GitHub Workflows说明 ,可以看下,如果有不明白的我再优化文档。

另外,PR title要加个前缀哈,feat: implement sequencer API with postgresql 这样

@seeflood
Copy link
Member

image
每次 CI 报错,可以点按钮查看详情
比如你现在这个报错 看起来是因为没格式化,可以本地 make all一下
image

@seeflood seeflood mentioned this pull request May 14, 2022
2 tasks
@azhsmesos
Copy link
Contributor Author

image 每次 CI 报错,可以点按钮查看详情 比如你现在这个报错 看起来是因为没格式化,可以本地 make all一下 image

感谢,我这次弄好在提pr吧

@azhsmesos azhsmesos changed the title implement sequencer API with postgresql feat: implement sequencer API with postgresql May 14, 2022
@azhsmesos
Copy link
Contributor Author

hello 帮下我running workflows~

@azhsmesos
Copy link
Contributor Author

又commit了一下,刚刚看报错是由于我提供的sql脚本中并没有声明有效的许可证头,不过sql文件注释是 -- ,而许可证那个是// ,不过我已经在sql文件中加上了,sql文件主要是提供sql表脚本,不在项目中具体执行,虽然在sql文件中使用 // 不符合常理,不过应该没啥影响吧,主要为了跑ci,麻烦在帮忙运行一下,如果在失败我就把sql脚本 删掉,这次不上传sql 脚本了

@codecov
Copy link

codecov bot commented May 23, 2022

Codecov Report

Merging #567 (4626a4c) into main (3565211) will decrease coverage by 0.85%.
The diff coverage is 0.00%.

❗ Current head 4626a4c differs from pull request most recent head d751d95. Consider uploading reports for the commit d751d95 to get more accurate results

@@            Coverage Diff             @@
##             main     #567      +/-   ##
==========================================
- Coverage   60.70%   59.84%   -0.86%     
==========================================
  Files         120      122       +2     
  Lines        6382     6473      +91     
==========================================
  Hits         3874     3874              
- Misses       2139     2230      +91     
  Partials      369      369              
Impacted Files Coverage Δ
components/pkg/utils/postgresql.go 0.00% <0.00%> (ø)
...nents/sequencer/postgresql/postgresql_sequencer.go 0.00% <0.00%> (ø)
sdk/go-sdk/client/invoke.go 78.66% <ø> (ø)
sdk/go-sdk/client/state.go 75.49% <ø> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 54f034f...d751d95. Read the comment docs.

@seeflood seeflood closed this May 26, 2022
@seeflood seeflood reopened this May 26, 2022
@ZLBer
Copy link
Member

ZLBer commented May 27, 2022

@azhsmesos hi,some problems:

  1. why are there so many empty files?
  2. replace fmt.Print with logger
  3. using English comments
  4. do not use yaml config, we have config file under ./configs

you can take a look at the implementation of other sequencer components.
have fun.

@azhsmesos
Copy link
Contributor Author

@azhsmesos hi,some problems:

  1. why are there so many empty files?
  2. replace fmt.Print with logger
  3. using English comments
  4. do not use yaml config, we have config file under ./configs

you can take a look at the implementation of other sequencer components. have fun.

@ZLBer the empty file may be the image file generated by me after startup, and there is residue when I delete it
and I will change other code styles, thank you

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants