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.

Sign up for GitHub

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

Jump to bottom

[master < ] Fixes for 100% percent cpu issues #1124

Merged
merged 2 commits into from Aug 11, 2023
Merged

[master < ] Fixes for 100% percent cpu issues #1124

merged 2 commits into from Aug 11, 2023

Conversation

Ignition
Copy link
Contributor

@Ignition Ignition commented Jul 31, 2023 •
edited

[master < Task] PR

  • Check, and update documentation if necessary
  • Provide the full content or a guide for the final git message

To keep docs changelog up to date, one more thing to do:

  • Write a release note here, including added/changed clauses

Added a delta cache to improve query performance. This helps in situations of repeated reads of vertices which have many delta changes cause by another transaction while the current transactions is operating with snapshot isolation level and so needs to process those deltas. This can be tuned using --delta-chain-cache-threashold

  • Tag someone from docs team in the comments

relates to #865

@Ignition Ignition marked this pull request as draft July 31, 2023 09:33
@gitbuda gitbuda added this to the mg-v2.11.0 milestone Aug 1, 2023
@gitbuda gitbuda mentioned this pull request Aug 1, 2023
Closed
4 tasks
@Ignition Ignition force-pushed the MG_865_100_percent_CPU branch 4 times, most recently from 31760e9 to f9e1530 Compare August 4, 2023 16:07
andrejtonev
andrejtonev reviewed Aug 7, 2023
View reviewed changes
Copy link
Contributor

@andrejtonev andrejtonev left a comment

Choose a reason for hiding this comment

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

First pass

src/storage/v2/edge_direction.hpp Outdated Show resolved Hide resolved
src/storage/v2/vertex_info_cache.cpp Outdated Show resolved Hide resolved
src/storage/v2/vertex_info_cache.cpp Outdated Show resolved Hide resolved
src/storage/v2/vertex_info_cache.cpp Outdated Show resolved Hide resolved
src/storage/v2/vertex_info_cache.cpp Outdated Show resolved Hide resolved
src/storage/v2/vertex_info_cache.cpp Show resolved Hide resolved
src/storage/v2/vertex_info_helpers.hpp Show resolved Hide resolved
src/storage/v2/vertex_info_helpers.hpp Show resolved Hide resolved
src/storage/v2/vertex_info_helpers.hpp Outdated Show resolved Hide resolved
@Ignition Ignition force-pushed the MG_865_100_percent_CPU branch 2 times, most recently from 53b594b to af27ad4 Compare August 8, 2023 12:08
@Ignition Ignition marked this pull request as ready for review August 8, 2023 12:08
andrejtonev
andrejtonev reviewed Aug 8, 2023
View reviewed changes
Copy link
Contributor

@andrejtonev andrejtonev left a comment

Choose a reason for hiding this comment

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

Great work!
Just a couple of extra comments.

Could we add tests as well?

src/storage/v2/inmemory/indices_utils.hpp Outdated Show resolved Hide resolved
src/storage/v2/mvcc.hpp Show resolved Hide resolved
src/storage/v2/inmemory/indices_utils.hpp Outdated Show resolved Hide resolved
src/storage/v2/vertex_info_cache.hpp Outdated Show resolved Hide resolved
@Ignition Ignition mentioned this pull request Aug 8, 2023
Open
@Ignition
Copy link
Contributor Author

Ignition commented Aug 8, 2023

@andrejtonev I'm going to delay delivering benchmark until #1152 is done.

@Ignition Ignition force-pushed the MG_865_100_percent_CPU branch 2 times, most recently from 73900d2 to 8e1fa4e Compare August 9, 2023 10:55
Josipmrden
Josipmrden reviewed Aug 9, 2023
View reviewed changes
Copy link
Contributor

@Josipmrden Josipmrden left a comment

Choose a reason for hiding this comment

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

Some comments, will do an additional review afterwards

.pre-commit-config.yaml Show resolved Hide resolved
src/storage/v2/inmemory/indices_utils.hpp Show resolved Hide resolved
src/storage/v2/inmemory/storage.cpp Show resolved Hide resolved
src/storage/v2/vertex_accessor.cpp Show resolved Hide resolved
@Josipmrden
Copy link
Contributor

Going to review the correctness by testing it myself now

@Ignition
Simplify skip_list
a5d3254 
Put `gen_hight()` as a static method in a non-templated base class. Also
make the random number generator thread_local to remove the need for a
spin lock.

In SkipListGc, changed some const data members for static constexpr so
that they do not take any space.
@Ignition Ignition force-pushed the MG_865_100_percent_CPU branch 3 times, most recently from a70bf35 to 47db683 Compare August 10, 2023 12:42
@Josipmrden Josipmrden merged commit 2e51e70 into master Aug 11, 2023
6 checks passed
@Josipmrden Josipmrden deleted the MG_865_100_percent_CPU branch August 11, 2023 08:18
gitbuda pushed a commit that referenced this pull request Aug 21, 2023
@Ignition @gitbuda
Add supernode vertex cache (#1124)
ce609f6 
Add supernode vertex cache to account for long delta chains and modifications in the same module being independent of scanning of the nodes in the next iteration of the pulling mechanism.
@gitbuda gitbuda mentioned this pull request Aug 22, 2023
Closed
@vpavicic vpavicic added the Docs needed Docs needed label Sep 12, 2023
as51340 pushed a commit that referenced this pull request Nov 10, 2023
@Ignition @as51340
Add supernode vertex cache (#1124)
c9dc84c 
Add supernode vertex cache to account for long delta chains and modifications in the same module being independent of scanning of the nodes in the next iteration of the pulling mechanism.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Josipmrden Josipmrden Josipmrden left review comments

@antoniofilipovic antoniofilipovic antoniofilipovic left review comments

@andrejtonev andrejtonev andrejtonev approved these changes

Assignees

@Ignition Ignition

Labels
Docs needed Docs needed
Projects
None yet
Milestone
mg-v2.11.0
Development

Successfully merging this pull request may close these issues.

None yet
6 participants
@Ignition @Josipmrden @andrejtonev @antoniofilipovic @gitbuda @vpavicic