Skip to content
This repository has been archived by the owner on Sep 18, 2023. It is now read-only.

[NSE-516]Support ExternalSorter to control memory footage #517

Merged
merged 8 commits into from
Sep 29, 2021

Conversation

xuechendi
Copy link
Collaborator

  • 2021/09/23
    CPP part functionality test is done, all four cases are supported

  • Next

  1. add benchmark test for memory and performance evaluation
  2. add java support for memory releasing

@github-actions
Copy link

#516

@xuechendi xuechendi force-pushed the wip_externalsort branch 4 times, most recently from 15ef1c2 to c12d400 Compare September 24, 2021 03:54
Signed-off-by: Chendi Xue <chendi.xue@intel.com>
… spill and read codes

Signed-off-by: Chendi Xue <chendi.xue@intel.com>
Signed-off-by: Chendi Xue <chendi.xue@intel.com>
Signed-off-by: Chendi Xue <chendi.xue@intel.com>
1. spill during evaluate
2. spill during resultIter
3. need to add lock to avoid threading fail or inconsistency

Signed-off-by: Chendi Xue <chendi.xue@intel.com>
@xuechendi xuechendi changed the title [NSE-516][DNM]Support ExternalSorter to control memory footage [NSE-516]Support ExternalSorter to control memory footage Sep 27, 2021
Signed-off-by: Chendi Xue <chendi.xue@intel.com>
Signed-off-by: Chendi Xue <chendi.xue@intel.com>
Comment on lines +2833 to +2834
RETURN_NOT_OK(impl_->Evaluate(in));
RETURN_NOT_OK(impl_->MaybeSpill());
Copy link
Collaborator

Choose a reason for hiding this comment

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

Do this 2 lines acquire for memory from context memory pool too?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

MaybeSpill will check if cached data go beyond threshold, and then do spill and read the first batch from disk, so there maybe some allocation happened.

Copy link
Collaborator

Choose a reason for hiding this comment

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

So this may cause deadlock? Somewhere seemed hanging when I was testing the code. I made sure it was not a Spill->ReadBatch->Spill call. maybe it was MaybeSpill->ReadBatch->Spill

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

maybespill is spill

Signed-off-by: Chendi Xue <chendi.xue@intel.com>
@xuechendi xuechendi merged commit 5aea527 into oap-project:master Sep 29, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants