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

Pipe: release resources of pipe event that have been GCed but the reference count is not zero by PhantomReference #13360

Merged
merged 31 commits into from
Oct 13, 2024

Conversation

VGalaxies
Copy link
Contributor

@VGalaxies VGalaxies commented Aug 31, 2024

Considering that when the event is garbage collected, the file resources it holds will not automatically end their lifecycle, we adopt a reference counting maintenance + safety net fallback method (releasing resources later is better than never releasing them).

@SteveYurongSu SteveYurongSu self-assigned this Sep 2, 2024
@VGalaxies VGalaxies marked this pull request as ready for review September 19, 2024 07:46
return PIPE_EVENT_PHANTOM_REFERENCES.size();
}

protected void gcHook() {
Copy link
Member

Choose a reason for hiding this comment

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

Use a seperated thread pool (extends PipePeriodicalJobExecutor?) to run this method periodically?

Copy link
Member

Choose a reason for hiding this comment

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

For one thread, what is the max number of events per second can be checked?

Copy link
Member

Choose a reason for hiding this comment

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

We have to consider using a multi-threaded pool to run this method if necessary (maybe we can add a conf parma for this?)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

consider optimizing in the next PR...

config.setPipeEventReferenceEliminateIntervalSeconds(
Long.parseLong(
properties.getProperty(
"pipe_event_reference_eliminate_interval_seconds",
Copy link
Member

Choose a reason for hiding this comment

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

Can be shorter by default?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

no other ideas for now...

@SteveYurongSu SteveYurongSu merged commit 29ae643 into apache:master Oct 13, 2024
27 of 28 checks passed
@VGalaxies VGalaxies deleted the PipePhantomReference branch October 13, 2024 07:26
VGalaxies added a commit to VGalaxies/iotdb that referenced this pull request Oct 13, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants