You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@storm.apache.org by HeartSaVioR <gi...@git.apache.org> on 2018/01/25 06:27:43 UTC
[GitHub] storm pull request #2532: STORM-2912 Revert optimization of sharing tick tup...
GitHub user HeartSaVioR opened a pull request:
https://github.com/apache/storm/pull/2532
STORM-2912 Revert optimization of sharing tick tuple
* since it incurs side effect and messes metrics
It must be cherry-picked into 1.1.x-branch and 1.0.x-branch as well.
Will also craft a patch for master branch shortly.
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/HeartSaVioR/storm STORM-2912-1.x
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/storm/pull/2532.patch
To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:
This closes #2532
----
commit 43f6d4883c229a129e60208a3e17f3d77d1cc563
Author: Jungtaek Lim <ka...@...>
Date: 2018-01-25T06:25:01Z
STORM-2912 Revert optimization of sharing tick tuple
* since it incurs side effect and messes metrics
----
---
[GitHub] storm issue #2532: STORM-2912 Revert optimization of sharing tick tuple (1.x...
Posted by ptgoetz <gi...@git.apache.org>.
Github user ptgoetz commented on the issue:
https://github.com/apache/storm/pull/2532
+1 Nice catch. Agree on performance -- tick tuples are sent infrequently enough that the optimization isn't necessary.
---
[GitHub] storm issue #2532: STORM-2912 Revert optimization of sharing tick tuple (1.x...
Posted by HeartSaVioR <gi...@git.apache.org>.
Github user HeartSaVioR commented on the issue:
https://github.com/apache/storm/pull/2532
@danny0405
If it's in critical path, it could bring perf. hit, not only from creating object but also from GC. We are targeting to emit millions of tuples in a second.
In this case, given that time unit for recurring is second, I'm sure that we don't have any perf. hit.
---
[GitHub] storm pull request #2532: STORM-2912 Revert optimization of sharing tick tup...
Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:
https://github.com/apache/storm/pull/2532
---
[GitHub] storm issue #2532: STORM-2912 Revert optimization of sharing tick tuple (1.x...
Posted by satishd <gi...@git.apache.org>.
Github user satishd commented on the issue:
https://github.com/apache/storm/pull/2532
@HeartSaVioR Was there a reasonable perf hit because of creating tick tuple instance whenever it is executed in timer?
---
[GitHub] storm issue #2532: STORM-2912 Revert optimization of sharing tick tuple (1.x...
Posted by HeartSaVioR <gi...@git.apache.org>.
Github user HeartSaVioR commented on the issue:
https://github.com/apache/storm/pull/2532
@satishd added comments. Commit amended because it is tiny change.
---
[GitHub] storm issue #2532: STORM-2912 Revert optimization of sharing tick tuple (1.x...
Posted by satishd <gi...@git.apache.org>.
Github user satishd commented on the issue:
https://github.com/apache/storm/pull/2532
@HeartSaVioR It may be helpful to have a comment on why it was changed with JIRA number so that same change is avoided later.
---
[GitHub] storm issue #2532: STORM-2912 Revert optimization of sharing tick tuple
Posted by HeartSaVioR <gi...@git.apache.org>.
Github user HeartSaVioR commented on the issue:
https://github.com/apache/storm/pull/2532
It is really hard to think such side effect while applying optimization like this. It is fairly easy to think that tuple is immutable, and it is true for interface `Tuple`, but no longer true for `TupleImpl`. Ideally `TupleImpl` should be also immutable.
Due to how acker works, we allow updating XOR value to `TupleImpl`, and due to how we measure metrics, we allow setting timestamp to `TupleImpl`.
It would be better if we have some way to address without incurring performance hit.
---
[GitHub] storm issue #2532: STORM-2912 Revert optimization of sharing tick tuple (1.x...
Posted by HeartSaVioR <gi...@git.apache.org>.
Github user HeartSaVioR commented on the issue:
https://github.com/apache/storm/pull/2532
@satishd @danny0405
Sorry guys. I missed while commenting, corrected now. **I don't think it brings any perf hit.** I wrote it opposite way. ;)
---
[GitHub] storm issue #2532: STORM-2912 Revert optimization of sharing tick tuple (1.x...
Posted by danny0405 <gi...@git.apache.org>.
Github user danny0405 commented on the issue:
https://github.com/apache/storm/pull/2532
@HeartSaVioR
Create an object is very lightweight, is should not have any performance hit.
---
[GitHub] storm issue #2532: STORM-2912 Revert optimization of sharing tick tuple (1.x...
Posted by arunmahadevan <gi...@git.apache.org>.
Github user arunmahadevan commented on the issue:
https://github.com/apache/storm/pull/2532
+1
---
[GitHub] storm issue #2532: STORM-2912 Revert optimization of sharing tick tuple (1.x...
Posted by HeartSaVioR <gi...@git.apache.org>.
Github user HeartSaVioR commented on the issue:
https://github.com/apache/storm/pull/2532
@satishd I don't know about the perf hit, and I don't think it doesn't bring actual perf hit, but anyone would think it is just weird to create same tuple every time, so easy to break it again.
---