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(server): enhance transaction in graph server #2686

Open
wants to merge 10 commits into
base: master
Choose a base branch
from

Conversation

Aph-CUG
Copy link

@Aph-CUG Aph-CUG commented Oct 25, 2024

Purpose of the PR

  • close #xxx

Main Changes

Verifying these changes

  • Trivial rework / code cleanup without any test coverage. (No Need)
  • Already covered by existing tests, such as (please modify tests here).
  • Need tests and can be verified as follows:
    • xxx

Does this PR potentially affect the following parts?

Documentation Status

  • Doc - TODO
  • Doc - Done
  • Doc - No Need

@dosubot dosubot bot added the size:XL This PR changes 500-999 lines, ignoring generated files. label Oct 25, 2024
@imbajin imbajin changed the title 开源夏令营——事务型增强 feat(server): enhance transaction in graph server Oct 25, 2024
Copy link
Member

Choose a reason for hiding this comment

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

Can we add more context for the design? (investigation/effect -> before / after)

Copy link
Member

Choose a reason for hiding this comment

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

image

Also fix the little conflict in code & run CI

@@ -0,0 +1,239 @@
## 事务日志——checklist
Copy link
Contributor

Choose a reason for hiding this comment

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

@imbajin 设计文档可以放到doc仓库,另外可以将之前文档仓库也统一放到一个地方?

Copy link
Member

@imbajin imbajin Oct 28, 2024

Choose a reason for hiding this comment

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

@imbajin 设计文档可以放到doc仓库,另外可以将之前文档仓库也统一放到一个地方?

(all) move to doc repo maybe (add "blog/design doc" column for them)

Comment on lines 73 to 74
} catch (Throwable e) {
throw e;
Copy link
Contributor

Choose a reason for hiding this comment

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

don't need to catch if we just re-throw here?

switch (checkList.getStage()) {
case "config": {
configContinue(checkList);
}
Copy link
Contributor

Choose a reason for hiding this comment

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

can we add some comments that we don't expect break here and explain why

@imbajin
Copy link
Member

imbajin commented Oct 31, 2024

@Aph-CUG maybe we need to handle the CI error
image

Aph-CUG and others added 4 commits November 1, 2024 06:32
# Conflicts:
#	hugegraph-server/hugegraph-dist/src/main/java/org/apache/hugegraph/TxScanner.java
@dosubot dosubot bot added size:L This PR changes 100-499 lines, ignoring generated files. and removed size:XL This PR changes 500-999 lines, ignoring generated files. labels Nov 1, 2024
@Aph-CUG
Copy link
Author

Aph-CUG commented Nov 1, 2024

@Aph-CUG maybe we need to handle the CI error image

ok,我最近完善好这个pr

Copy link

github-actions bot commented Dec 1, 2024

Due to the lack of activity, the current pr is marked as stale and will be closed after 180 days, any update will remove the stale label

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
inactive size:L This PR changes 100-499 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants