[GitHub] storm pull request: [STORM-636] Faster, optional retrieval of last...
Github user asfgit closed the pull request at: https://github.com/apache/storm/pull/392 --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] storm pull request: [STORM-636] Faster, optional retrieval of last...
Github user revans2 commented on the pull request: https://github.com/apache/storm/pull/392#issuecomment-71864014 I agree that ZK is not the ideal place to storm most things, but it is by far the most convenient. As such unless it is shown that it is causing a significant load on ZK I would rather leave it there until we can find/build a better place to put it. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] storm pull request: [STORM-636] Faster, optional retrieval of last...
Github user d2r commented on the pull request: https://github.com/apache/storm/pull/392#issuecomment-71865651 Agreed, I would like a review from @Parth-Brahmbhatt first if possible. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] storm pull request: [STORM-636] Faster, optional retrieval of last...
Github user revans2 commented on the pull request: https://github.com/apache/storm/pull/392#issuecomment-71864756 The code looks fine to me +1. Although I want to wait for @Parth-Brahmbhatt to finish his review before merging anything in. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] storm pull request: [STORM-636] Faster, optional retrieval of last...
Github user Parth-Brahmbhatt commented on the pull request: https://github.com/apache/storm/pull/392#issuecomment-71706395 I did not know we only stored 10 errors, makes sense to not go with pagination if its hardcoded to 10. I will take a look at the complete PR today. Not to derail the discussion but personally, I would much rather not store errors in zk at all if its just for rendering the errors in UI. If the spouts/bolts could just store this in memory with some expiration that should suffice and we could expose an API at worker layer to get this information directly from it. If the host dies you lose some errors but that does not seem like a big deal. The only downside will be ui would now have to make requests against worker hosts to get erros but that seems ok to me, you would also get parallelism as all these worker calls can be made in parallel. I haven't thought this through completely and its probably much more work but I would love to hear your opinion. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] storm pull request: [STORM-636] Faster, optional retrieval of last...
Github user d2r commented on a diff in the pull request:
https://github.com/apache/storm/pull/392#discussion_r23627700
--- Diff: storm-core/src/storm.thrift ---
@@ -243,6 +243,16 @@ struct SubmitOptions {
2: optional Credentials creds;
}
+enum NumErrorsChoice {
+ ALL,
+ NONE,
+ ONE
+}
+
+struct GetInfoOptions {
+ 1: optional NumErrorsChoice num_err_choice;
--- End diff --
I hadn't thought about it that way, but I'm open to it. Could you give an
example of what you mean?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] storm pull request: [STORM-636] Faster, optional retrieval of last...
GitHub user d2r opened a pull request: https://github.com/apache/storm/pull/392 [STORM-636] Faster, optional retrieval of last component error We want to speed up interactions with the topology that call getTopologyInfo, including the Topology Page in the UI. Before change: - Errors written to /errors/topo-id/comp-name/eN, where N is a sequence number. - getTopologyInfo grabs all /errors/topo-id/*/* errors from ZK After this change: - Errors written to /errors/topo-id/comp-name/eN, where N is a sequence number. (unchanged) - Errors also written to /errors/topo-id/comp-name-last-error (this means there is an extra ZK write per error) - New Nimbus thrift interface method `getTopologyInfoWithOpts` - Only option right now is how many errors to retrieve from ZK: 0, 1, or all. You can merge this pull request into a Git repository by running: $ git pull https://github.com/d2r/storm storm-636-ui-errors Alternatively you can review and apply these changes as the patch at: https://github.com/apache/storm/pull/392.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 #392 commit 1cfa190f2efb06f8798984b43dec801e5ff20ad5 Author: Derek Dagit der...@yahoo-inc.com Date: 2015-01-22T16:46:03Z Faster, optional retrieval of last component error --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] storm pull request: [STORM-636] Faster, optional retrieval of last...
Github user Parth-Brahmbhatt commented on a diff in the pull request:
https://github.com/apache/storm/pull/392#discussion_r23393694
--- Diff: storm-core/src/storm.thrift ---
@@ -243,6 +243,16 @@ struct SubmitOptions {
2: optional Credentials creds;
}
+enum NumErrorsChoice {
+ ALL,
+ NONE,
+ ONE
+}
+
+struct GetInfoOptions {
+ 1: optional NumErrorsChoice num_err_choice;
--- End diff --
Instead of an Enum don't you think a pagination struct with a start and end
would be more flexible?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
