Skip to content

Intern ServerInventoryView inventory objects.#3238

Merged
nishantmonu51 merged 1 commit intoapache:masterfrom
metamx:internInventory
Jul 14, 2016
Merged

Intern ServerInventoryView inventory objects.#3238
nishantmonu51 merged 1 commit intoapache:masterfrom
metamx:internInventory

Conversation

@drcrallen
Copy link
Copy Markdown
Contributor

@drcrallen drcrallen commented Jul 13, 2016

Allows replicants of data segments to only take up one chunklet of heap in the server view.

Related to #3237

@drcrallen drcrallen added this to the 0.9.2 milestone Jul 13, 2016
@drcrallen
Copy link
Copy Markdown
Contributor Author

I'm working on getting this rolled out internally

@drcrallen
Copy link
Copy Markdown
Contributor Author

drcrallen commented Jul 13, 2016

screen shot 2016-07-13 at 9 37 26 am

Results are promising so far. With this patch tending to lead to ~50% reduction in old-gen (java heap%) size as time progresses.

Orange is this patch, Blue is a node without this patch. Both were restarted at nearly the same time (as close as rolling-restarts can get you)

@drcrallen drcrallen changed the title Optionally intern ServerInventoryView inventory objects. Intern ServerInventoryView inventory objects. Jul 13, 2016
@gianm
Copy link
Copy Markdown
Contributor

gianm commented Jul 13, 2016

Per #3237 (comment) it seems fair to do this in order to de-dupe replicas of the same segment regardless of what happens for #3237.

For #3237 IMO this shouldn't be considered a fix, we should actually collapse the two server views into one.

@drcrallen
Copy link
Copy Markdown
Contributor Author

@gianm agreed on not a fix. I'm hoping "Related" doesn't count as "Fixed" in github-land-auto-closing

@gianm
Copy link
Copy Markdown
Contributor

gianm commented Jul 13, 2016

👍

@nishantmonu51
Copy link
Copy Markdown
Member

👍

@nishantmonu51 nishantmonu51 merged commit a931deb into apache:master Jul 14, 2016
@drcrallen drcrallen deleted the internInventory branch July 14, 2016 03:39
drcrallen added a commit to metamx/druid that referenced this pull request Jul 26, 2016
…che#3238)"

This reverts commit a931deb.
Fixes apache#3283

The core issue here is that realtime nodes announce their size as 0, so a coordinator which interns the realtime version of the data segment will not be able to see the new sized announcement when handoff occurs.

This is caused by the `eauals` method on a `DataSegment` only evaluating the identifier. the `eauals` method *should* be correct for object equivalence, and things which need to check equivalence of some sub-portion of the object should do so explicitly.
drcrallen added a commit to metamx/druid that referenced this pull request Jul 26, 2016
…che#3238)"

This reverts commit a931deb.
Fixes apache#3283

The core issue here is that realtime nodes announce their size as 0, so a coordinator which interns the realtime version of the data segment will not be able to see the new sized announcement when handoff occurs.

This is caused by the `eauals` method on a `DataSegment` only evaluating the identifier. the `eauals` method *should* be correct for object equivalence, and things which need to check equivalence of some sub-portion of the object should do so explicitly.
fjy pushed a commit that referenced this pull request Jul 26, 2016
…)" (#3286)

This reverts commit a931deb.
Fixes #3283

The core issue here is that realtime nodes announce their size as 0, so a coordinator which interns the realtime version of the data segment will not be able to see the new sized announcement when handoff occurs.

This is caused by the `eauals` method on a `DataSegment` only evaluating the identifier. the `eauals` method *should* be correct for object equivalence, and things which need to check equivalence of some sub-portion of the object should do so explicitly.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants