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

Releasable bytes output + use in transport / translog #5691

Closed
wants to merge 2 commits into from

Conversation

@kimchy
Copy link
Member

commented Apr 4, 2014

create a new releasable bytes output, that can be recycled, and use it in netty and the translog, 2 areas where the recycling will help nicely.
Note, opted for statically typed enforced releasble bytes output, to make sure people take the extra care to control when the bytes reference are released.
Also, the mock page/array classes were fixed to not take into account potential recycling going during teardown, for example, on a shared cluster ping requests still happen, so recycling happen actively during teardown.

create a new releasable bytes output, that can be recycled, and use it in netty and the translog, 2 areas where the recycling will help nicely.
Note, opted for statically typed enforced releasble bytes output, to make sure people take the extra care to control when the bytes reference are released.
 Also, the mock page/array classes were fixed to not take into account potential recycling going during teardown, for example, on a shared cluster ping requests still happen, so recycling happen actively during teardown.
if (!failures.isEmpty()) {
final Object cause = failures.entrySet().iterator().next().getValue();
throw new RuntimeException(failures.size() + " arrays have not been released", cause instanceof Throwable ? (Throwable) cause : null);
}

This comment has been minimized.

Copy link
@jpountz

jpountz Apr 4, 2014

Contributor

I think this can be made easier to read and more concise using set operations like retainAll?

This comment has been minimized.

Copy link
@jpountz

jpountz Apr 4, 2014

Contributor

And could we extract the logic to an utility method so that both MockBigArrays and MockPageCacheRecycler can use it?

releasable.release();
if (released.compareAndSet(false, true)) {
releasable.release();
}

This comment has been minimized.

Copy link
@jpountz

jpountz Apr 4, 2014

Contributor

what motivated this change? did you hit a double release issue?

This comment has been minimized.

Copy link
@kimchy

kimchy Apr 4, 2014

Author Member

I used to use this listener in 2 listeners while I was tracking the mock failures, but once fixed the mock failures, decided to keep it as this is a generic listener. Can remove it for now, will do.

@@ -53,7 +53,8 @@
try {
localAddressX = InetAddress.getLocalHost();
} catch (UnknownHostException e) {
logger.trace("Failed to find local host", e);
logger.trace("Failed to find local host, using loopback", e);
localAddressX = InetAddress.getLoopbackAddress();

This comment has been minimized.

Copy link
@jpountz

jpountz Apr 4, 2014

Contributor

should it be a separate change?

This comment has been minimized.

Copy link
@kimchy

kimchy Apr 4, 2014

Author Member

yea, it could..., will do

@jpountz

This comment has been minimized.

Copy link
Contributor

commented Apr 4, 2014

This looks good to me, I left comments but they are more about cosmetics.

- move NetworkUtils change, do it in separate change
- simplify set checks
- didn't refactor to a common class, It think the logic is small enough to not warrant it for now
@kimchy kimchy closed this in d26a956 Apr 6, 2014
@kimchy kimchy added doc and removed doc labels Apr 6, 2014
kimchy added a commit that referenced this pull request Apr 6, 2014
create a new releasable bytes output, that can be recycled, and use it in netty and the translog, 2 areas where the recycling will help nicely.
Note, opted for statically typed enforced releasble bytes output, to make sure people take the extra care to control when the bytes reference are released.
 Also, the mock page/array classes were fixed to not take into account potential recycling going during teardown, for example, on a shared cluster ping requests still happen, so recycling happen actively during teardown.
closes #5691
@kimchy kimchy deleted the kimchy:recycled_bytes_reference branch Apr 6, 2014
@clintongormley clintongormley changed the title releasable bytes output + use in transport / translog Releasable bytes output + use in transport / translog Jun 7, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants
You can’t perform that action at this time.