You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@druid.apache.org by GitBox <gi...@apache.org> on 2021/05/17 11:14:00 UTC
[GitHub] [druid] FrankChen021 opened a new pull request #11262: Improve doc of movingAverage
FrankChen021 opened a new pull request #11262:
URL: https://github.com/apache/druid/pull/11262
### Description
There're some questions about how to enable movingAverage on brokers(eg #11086), this PR improves the doc to make operational steps more directive and also adds a limitation that movingAverage is not supported by druid router.
This PR has:
- [X] been self-reviewed.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org
[GitHub] [druid] bananaaggle commented on pull request #11262: Improve doc of movingAverage
Posted by GitBox <gi...@apache.org>.
bananaaggle commented on pull request #11262:
URL: https://github.com/apache/druid/pull/11262#issuecomment-844622124
> ### Description
> There're some questions about how to enable movingAverage on brokers(eg #11086), this PR improves the doc to make operational steps more directive and also adds a limitation that movingAverage is not supported by druid router.
>
> This PR has:
>
> * [x] been self-reviewed.
Actually, add binder.bind(c.class).to(FakeQuerySegmentWalker.class).in(LazySingleton.class) to CliRouter.java, users can query movingAverage through router. One line code can break this limit, but it is not very graceful because router not use QuerySegmentWalker.class.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org
[GitHub] [druid] bananaaggle commented on pull request #11262: Improve doc of movingAverage
Posted by GitBox <gi...@apache.org>.
bananaaggle commented on pull request #11262:
URL: https://github.com/apache/druid/pull/11262#issuecomment-844638244
> > > ### Description
> > > There're some questions about how to enable movingAverage on brokers(eg #11086), this PR improves the doc to make operational steps more directive and also adds a limitation that movingAverage is not supported by druid router.
> > > This PR has:
> > >
> > > * [x] been self-reviewed.
> >
> >
> > Actually, add binder.bind(c.class).to(FakeQuerySegmentWalker.class).in(LazySingleton.class) to CliRouter.java, users can query movingAverage through router. One line code can break this limit, but it is not very graceful because router not use QuerySegmentWalker.class.
>
> Thanks for pointing out this. Yes, we need to make some code change to support moving average running from router. This is not what this PR aims for. I'm planning to do it later or if you're interested in this feature, you could take it.
Last June I do this adjust on my cluster for using moving average for analysts. It works, but not graceful. Do you think it is a suitable idea to solve this problem? If you think it's OK, I can test it on master branch and open PR. It's very easy and I'm glad to do this.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org
[GitHub] [druid] FrankChen021 commented on pull request #11262: Improve doc of movingAverage
Posted by GitBox <gi...@apache.org>.
FrankChen021 commented on pull request #11262:
URL: https://github.com/apache/druid/pull/11262#issuecomment-850047083
> Travis seems to be failing on spellcheck for `pull-deps` and I can't figure out why, it seems like this is used in other files without a problem. 🤔
Sorry, I forgot the CI. A suppression rule has been added to the `.spelling` file, and now let's wait to see the result of CI.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org
[GitHub] [druid] bananaaggle commented on pull request #11262: Improve doc of movingAverage
Posted by GitBox <gi...@apache.org>.
bananaaggle commented on pull request #11262:
URL: https://github.com/apache/druid/pull/11262#issuecomment-844621303
> ### Description
> There're some questions about how to enable movingAverage on brokers(eg #11086), this PR improves the doc to make operational steps more directive and also adds a limitation that movingAverage is not supported by druid router.
>
> This PR has:
>
> * [x] been self-reviewed.
Actually, add binder.bind(c.class).to(FakeQuerySegmentWalker.class).in(LazySingleton.class) to CliRouter.java, user can query movingAverage through router. One line code can break this limit, but it is not very graceful because router not use QuerySegmentWalker.class.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org
[GitHub] [druid] suneet-s commented on pull request #11262: Improve doc of movingAverage
Posted by GitBox <gi...@apache.org>.
suneet-s commented on pull request #11262:
URL: https://github.com/apache/druid/pull/11262#issuecomment-850120524
> > Travis seems to be failing on spellcheck for pull-deps and I can't figure out why, it seems like this is used in other files without a problem. 🤔
>
> The .spelling file is sort of funny, the top of the file is like global suppressions, then below that there are page specific suppressions, which I'm not quite certain why they need to exist (do we need some things to be spelling errors on some pages but not on others), but explains why it was failing here, since pull-deps is suppressed in `docs/development/extensions.md` and `docs/development/extensions-contrib/redis-cache.md` (funny enough not in `docs/operations/pull-deps.md` which always refers to it with back ticks)
HA! that explains it. I don't think we need to make `pull-deps` a global suppression in this PR, so this change is good to go as is for me, although I wouldn't be opposed to a global `pull-deps` suppression either. @FrankChen021 I'll leave it to you to press the merge button for when you're happy with the change
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org
[GitHub] [druid] bananaaggle removed a comment on pull request #11262: Improve doc of movingAverage
Posted by GitBox <gi...@apache.org>.
bananaaggle removed a comment on pull request #11262:
URL: https://github.com/apache/druid/pull/11262#issuecomment-844621303
> ### Description
> There're some questions about how to enable movingAverage on brokers(eg #11086), this PR improves the doc to make operational steps more directive and also adds a limitation that movingAverage is not supported by druid router.
>
> This PR has:
>
> * [x] been self-reviewed.
Actually, add binder.bind(c.class).to(FakeQuerySegmentWalker.class).in(LazySingleton.class) to CliRouter.java, user can query movingAverage through router. One line code can break this limit, but it is not very graceful because router not use QuerySegmentWalker.class.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org
[GitHub] [druid] clintropolis commented on pull request #11262: Improve doc of movingAverage
Posted by GitBox <gi...@apache.org>.
clintropolis commented on pull request #11262:
URL: https://github.com/apache/druid/pull/11262#issuecomment-850055241
>Travis seems to be failing on spellcheck for pull-deps and I can't figure out why, it seems like this is used in other files without a problem. 🤔
The .spelling file is sort of funny, the top of the file is like global suppressions, then below that there are page specific suppressions, which I'm not quite certain why they need to exist (do we need some things to be spelling errors on some pages but not on others), but explains why it was failing here, since pull-deps is suppressed in `docs/development/extensions.md` and `docs/development/extensions-contrib/redis-cache.md` (funny enough not in `docs/operations/pull-deps.md` which always refers to it with back ticks)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org
[GitHub] [druid] FrankChen021 commented on pull request #11262: Improve doc of movingAverage
Posted by GitBox <gi...@apache.org>.
FrankChen021 commented on pull request #11262:
URL: https://github.com/apache/druid/pull/11262#issuecomment-844633988
> > ### Description
> > There're some questions about how to enable movingAverage on brokers(eg #11086), this PR improves the doc to make operational steps more directive and also adds a limitation that movingAverage is not supported by druid router.
> > This PR has:
> >
> > * [x] been self-reviewed.
>
> Actually, add binder.bind(c.class).to(FakeQuerySegmentWalker.class).in(LazySingleton.class) to CliRouter.java, users can query movingAverage through router. One line code can break this limit, but it is not very graceful because router not use QuerySegmentWalker.class.
Thanks for pointing out this. Yes, we need to make some code change to support moving average running from router. This is not what this PR aims for. I'm planning to do it later or if you're interested in this feature, you could take it.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org
[GitHub] [druid] suneet-s commented on pull request #11262: Improve doc of movingAverage
Posted by GitBox <gi...@apache.org>.
suneet-s commented on pull request #11262:
URL: https://github.com/apache/druid/pull/11262#issuecomment-849654283
Travis seems to be failing on spellcheck for `pull-deps` and I can't figure out why, it seems like this is used in other files without a problem. 🤔
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org
[GitHub] [druid] FrankChen021 merged pull request #11262: Improve doc of movingAverage
Posted by GitBox <gi...@apache.org>.
FrankChen021 merged pull request #11262:
URL: https://github.com/apache/druid/pull/11262
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org