Solr
  1. Solr
  2. SOLR-4030

Allow rate limiting Directory IO based on the IO context

    Details

    • Type: New Feature New Feature
    • Status: Closed
    • Priority: Minor Minor
    • Resolution: Fixed
    • Affects Version/s: None
    • Fix Version/s: 4.1, 6.0
    • Component/s: None
    • Labels:

      Description

      Allow setting rate limits for default,read,merge,flush.

        <directoryFactory name="DirectoryFactory" 
                          class="${solr.directoryFactory:solr.NRTCachingDirectoryFactory}"> 
                    <double name="maxWriteMBPerSecFlush">20</double>
                    <double name="maxWriteMBPerSecMerge">20</double>    
        </directoryFactory>
      

        Activity

        Hide
        Mark Miller added a comment -

        This would be nice to add - could you update this patch to 5x trunk Radmin?

        Show
        Mark Miller added a comment - This would be nice to add - could you update this patch to 5x trunk Radmin?
        Hide
        Radim Kolar added a comment -

        it still merges into trunk cleanly. just do git pull

        Show
        Radim Kolar added a comment - it still merges into trunk cleanly. just do git pull
        Hide
        Mark Miller added a comment -

        it still merges into trunk cleanly.

        Sure, it applies - it does not compile.

        just do git pull

        I'm using your patch and an svn checkout - no git to be found.

        Show
        Mark Miller added a comment - it still merges into trunk cleanly. Sure, it applies - it does not compile. just do git pull I'm using your patch and an svn checkout - no git to be found.
        Hide
        Radim Kolar added a comment -

        but it failed to build with 5.x in jenkins. i will take a more detailed look on it

        Show
        Radim Kolar added a comment - but it failed to build with 5.x in jenkins. i will take a more detailed look on it
        Hide
        Radim Kolar added a comment -

        LUCENE-4537 is cause. I will rework it but its not clear if new rate limiter is used for ordinary writes (not just merges) as well.

        Show
        Radim Kolar added a comment - LUCENE-4537 is cause. I will rework it but its not clear if new rate limiter is used for ordinary writes (not just merges) as well.
        Hide
        Radim Kolar added a comment -

        added limits for reading, merging and writing.

        Show
        Radim Kolar added a comment - added limits for reading, merging and writing.
        Hide
        Mark Miller added a comment -

        Looks good. I'll look at adding a unit test. I'd also rather detect when no settings have been entered (all unlimited) and not wrap the dir with a rate limiting wrapper in that case.

        We also do an instance of check in 4x against fsdir - so we should make that work with this.

        Show
        Mark Miller added a comment - Looks good. I'll look at adding a unit test. I'd also rather detect when no settings have been entered (all unlimited) and not wrap the dir with a rate limiting wrapper in that case. We also do an instance of check in 4x against fsdir - so we should make that work with this.
        Hide
        Radim Kolar added a comment -

        any progress on this? its trivial patch.

        Show
        Radim Kolar added a comment - any progress on this? its trivial patch.
        Hide
        Markus Jelsma added a comment -

        Why did you mark this as `not a problem`. This issue is not resolved.

        Show
        Markus Jelsma added a comment - Why did you mark this as `not a problem`. This issue is not resolved.
        Hide
        Radim Kolar added a comment -

        It was resolved for me. If you need this open your ticket.

        Show
        Radim Kolar added a comment - It was resolved for me. If you need this open your ticket.
        Hide
        Michael McCandless added a comment -

        Radim, maybe you can put your patch back?

        Other users could apply it / improve it, someone else may add a test case, etc., and it will eventually be committed. The process takes time ...

        Until Solr catches up, users can also look at ElasticSearch ... it's had IO throttling for a while now ( https://github.com/elasticsearch/elasticsearch/issues/2041 )

        Show
        Michael McCandless added a comment - Radim, maybe you can put your patch back? Other users could apply it / improve it, someone else may add a test case, etc., and it will eventually be committed. The process takes time ... Until Solr catches up, users can also look at ElasticSearch ... it's had IO throttling for a while now ( https://github.com/elasticsearch/elasticsearch/issues/2041 )
        Hide
        Radim Kolar added a comment -

        You guys had 1 month of time and wasted it. Now we fork and taking our patches with us. Case closed.

        Show
        Radim Kolar added a comment - You guys had 1 month of time and wasted it. Now we fork and taking our patches with us. Case closed.
        Hide
        Jack Krupansky added a comment -

        Now we fork

        Forking is understandable when you have time pressure and other interests to satisfy. No problem there. Hopefully you can contribute back some of the work from your fork.

        taking our patches with us.

        How is that related to forking? I mean, sure, you can apply the patch on your own fork, but why does forking imply that you think you need to delete the posted patch?

        Besides, didn't you cede ownership of the patch to the community/ASF when you posted it? So, technically, it is no longer yours, right?

        It sounds like you need a refresher course in "Community 101"!

        Show
        Jack Krupansky added a comment - Now we fork Forking is understandable when you have time pressure and other interests to satisfy. No problem there. Hopefully you can contribute back some of the work from your fork. taking our patches with us. How is that related to forking? I mean, sure, you can apply the patch on your own fork, but why does forking imply that you think you need to delete the posted patch? Besides, didn't you cede ownership of the patch to the community/ASF when you posted it? So, technically, it is no longer yours, right? It sounds like you need a refresher course in "Community 101"!
        Hide
        Radim Kolar added a comment -

        Unless you have written permission from us to distribute our work, you are losing court case against us after we prove to court that we are authors of code in question. We never lost such cases.

        Show
        Radim Kolar added a comment - Unless you have written permission from us to distribute our work, you are losing court case against us after we prove to court that we are authors of code in question. We never lost such cases.
        Hide
        Uwe Schindler added a comment -

        I will contact the board about what's going on here. Unfortunately, the custom JIRA plugin where you had to sign the ASF contribution by clicking the checkbox is no longer working with JIRA 5.2, now used by this server.

        Show
        Uwe Schindler added a comment - I will contact the board about what's going on here. Unfortunately, the custom JIRA plugin where you had to sign the ASF contribution by clicking the checkbox is no longer working with JIRA 5.2, now used by this server.
        Hide
        Markus Jelsma added a comment -

        We've had the same issues with him at Apache Nutch. After contacting the board it was decided to restore the original patch but not include it in the source tree, and ignore it further.

        Show
        Markus Jelsma added a comment - We've had the same issues with him at Apache Nutch. After contacting the board it was decided to restore the original patch but not include it in the source tree, and ignore it further.
        Hide
        Mark Miller added a comment -

        Apache has a pretty clear record in these cases I think - if a contributor wishes to withdraw his own work - especially if it's not yet committed to the codebase - we should simply allow them to do so. I'm sure situations around this can get complicated, but this case does not seem complicated at all. These patches have only one author and they have not been committed yet. We are in the business of accepting patches from willing contributors.

        If someone wants to see these features implemented, I'd suggested writing new patches. Neither issue is very large.

        Show
        Mark Miller added a comment - Apache has a pretty clear record in these cases I think - if a contributor wishes to withdraw his own work - especially if it's not yet committed to the codebase - we should simply allow them to do so. I'm sure situations around this can get complicated, but this case does not seem complicated at all. These patches have only one author and they have not been committed yet. We are in the business of accepting patches from willing contributors. If someone wants to see these features implemented, I'd suggested writing new patches. Neither issue is very large.
        Hide
        Uwe Schindler added a comment -

        Thanks Markus. So restoring patches in JIRA actually works with help of infra, but this is useless here, as we would not use it in our source tree. And: He said it is trivial, so anybody who has interest in this functionality could write the code easily. So I see no problem and we just leave the issue open until somebody has the time to resolve this with a good patch.

        I think we should ignore Radim for any patches about Lucene and Solr, his social competence seems to be zero. I added a filter to my mail inbox.

        Show
        Uwe Schindler added a comment - Thanks Markus. So restoring patches in JIRA actually works with help of infra, but this is useless here, as we would not use it in our source tree. And: He said it is trivial, so anybody who has interest in this functionality could write the code easily. So I see no problem and we just leave the issue open until somebody has the time to resolve this with a good patch. I think we should ignore Radim for any patches about Lucene and Solr, his social competence seems to be zero. I added a filter to my mail inbox.
        Hide
        Jack Krupansky added a comment -

        if a contributor wishes to withdraw his own work - especially if it's not yet committed to the codebase - we should simply allow them to do so.

        That makes sense at least for a short interval, except for the case of anyone who may have included the patch in their own fork and maybe even could be in production with it and is now in limbo or worse. In this specific instance the triviality and brief tenure of the patch kind of makes it moot, but for future instances now we have to think twice when recommending a non-committed patch. At a minimum, somewhere there needs to be a notice/warning that use and ownership of uncommitted patches is a potentially questionable and risky activity - and that permisssion can be revoked at any moment. And if the "donate" check box can't be restored, then there needs to be some mechanism for a donor to explicitly cede ownership, to at least confirm the donation even if its legal status may vary from jurisdiction to jurisdiction.

        Show
        Jack Krupansky added a comment - if a contributor wishes to withdraw his own work - especially if it's not yet committed to the codebase - we should simply allow them to do so. That makes sense at least for a short interval, except for the case of anyone who may have included the patch in their own fork and maybe even could be in production with it and is now in limbo or worse. In this specific instance the triviality and brief tenure of the patch kind of makes it moot, but for future instances now we have to think twice when recommending a non-committed patch. At a minimum, somewhere there needs to be a notice/warning that use and ownership of uncommitted patches is a potentially questionable and risky activity - and that permisssion can be revoked at any moment. And if the "donate" check box can't be restored, then there needs to be some mechanism for a donor to explicitly cede ownership, to at least confirm the donation even if its legal status may vary from jurisdiction to jurisdiction.
        Hide
        Uwe Schindler added a comment -

        Keep issue open

        Show
        Uwe Schindler added a comment - Keep issue open
        Hide
        Yonik Seeley added a comment -

        Lets not over-think this or make it too complicated guys...

        No, you can't take back a contribution once it's been contributed. If/when it's been committed or not is just a detail.
        We can normally choose not to commit it (the distinction is pretty important), and I think that's what we should do here.

        If a contribution wasn't valid in the first place (i.e. someone saying... "hey, this person didn't have permission to contribute X") then we can figure that out on a case-by-case basis. Hasn't happened yet here AFAIK.

        Show
        Yonik Seeley added a comment - Lets not over-think this or make it too complicated guys... No, you can't take back a contribution once it's been contributed. If/when it's been committed or not is just a detail. We can normally choose not to commit it (the distinction is pretty important), and I think that's what we should do here. If a contribution wasn't valid in the first place (i.e. someone saying... "hey, this person didn't have permission to contribute X") then we can figure that out on a case-by-case basis. Hasn't happened yet here AFAIK.
        Hide
        Uwe Schindler added a comment -

        @Yonik: I agree here, we should not overcomplicate it. Just ignore Radim (at least I will do this). My only problem here is the missing checkbox in JIRA, where the users from the beginning have to specify if they agree with the Apache License. In that case, Radim would not be able to go to court or think about it, because he agreed on publishing the patch by open source terms. The problem, as Jack now explain, is the case for external people, using this issue tracker as a source for their work. Maybe some company would have used Radim's patch for their own product! Nobody inform them that he removed his patch here.

        In my opinion, you should not be able to at least remove patches in JIRA, but just offer the option to say at a later stage: "I submitted the patch, but I need to undo that." This should be noted in the submit form, so anybody who is not sure if the patch can be added here, would not do it. If some patch really have to be deleted, only PMC members should be able to do this completely (like in SVN where you can revert, but you have no chance to completely remove the occurence - only SVN admins could really remove a commit completely).

        To stop Radim editing this issues or reopen/close it, I set a new reporter.

        Show
        Uwe Schindler added a comment - @Yonik: I agree here, we should not overcomplicate it. Just ignore Radim (at least I will do this). My only problem here is the missing checkbox in JIRA, where the users from the beginning have to specify if they agree with the Apache License. In that case, Radim would not be able to go to court or think about it, because he agreed on publishing the patch by open source terms. The problem, as Jack now explain, is the case for external people, using this issue tracker as a source for their work. Maybe some company would have used Radim's patch for their own product! Nobody inform them that he removed his patch here. In my opinion, you should not be able to at least remove patches in JIRA, but just offer the option to say at a later stage: "I submitted the patch, but I need to undo that." This should be noted in the submit form, so anybody who is not sure if the patch can be added here, would not do it. If some patch really have to be deleted, only PMC members should be able to do this completely (like in SVN where you can revert, but you have no chance to completely remove the occurence - only SVN admins could really remove a commit completely). To stop Radim editing this issues or reopen/close it, I set a new reporter.
        Hide
        Yonik Seeley added a comment -

        The checkbox was never necessary - it's just that people became used to seeing it and started assuming it was.
        Contributions to our software are under the ASL by default - one would need to explicitly state when adding something that looks like a contribution to our JIRA that it was in fact not a contribution (and that is what the old checkbox facilitated).

        Show
        Yonik Seeley added a comment - The checkbox was never necessary - it's just that people became used to seeing it and started assuming it was. Contributions to our software are under the ASL by default - one would need to explicitly state when adding something that looks like a contribution to our JIRA that it was in fact not a contribution (and that is what the old checkbox facilitated).
        Hide
        Uwe Schindler added a comment -

        Contributions to our software are under the ASL by default

        But does any user who opens an issue knows this? The Apache Issue tracker is missing a extra page, referred from the create issue / upload patch page that all work done here is under ASL. My own issue tracker presents the terms and conditions to the user when using the issue tracker.

        Show
        Uwe Schindler added a comment - Contributions to our software are under the ASL by default But does any user who opens an issue knows this? The Apache Issue tracker is missing a extra page, referred from the create issue / upload patch page that all work done here is under ASL. My own issue tracker presents the terms and conditions to the user when using the issue tracker.
        Hide
        Radim Kolar added a comment -

        Yes, i had similar issue with nutch project. Markus Jelsma reuploaded my patch back to JIRA and refused to delete it. Its still in JIRA violating my copyright laws. I decided not to escalate the conflict and go after ASF and harm other projects just because of misbehavior of one person.

        Show
        Radim Kolar added a comment - Yes, i had similar issue with nutch project. Markus Jelsma reuploaded my patch back to JIRA and refused to delete it. Its still in JIRA violating my copyright laws. I decided not to escalate the conflict and go after ASF and harm other projects just because of misbehavior of one person.
        Hide
        Erick Erickson added a comment -

        Radim:

        At some point, you might reflect upon the fact that the common element in your conflicts with various Apache projects is...you.

        Show
        Erick Erickson added a comment - Radim: At some point, you might reflect upon the fact that the common element in your conflicts with various Apache projects is...you.
        Hide
        Mark Miller added a comment - - edited

        Here is a simple patch that lets you use a rate limited directory wrapper - you can set the rate for each of the io contexts that lucene provides. If no limited are configured, no rate limited directory wrapper is created.

        Show
        Mark Miller added a comment - - edited Here is a simple patch that lets you use a rate limited directory wrapper - you can set the rate for each of the io contexts that lucene provides. If no limited are configured, no rate limited directory wrapper is created.
        Hide
        Commit Tag Bot added a comment -

        [trunk commit] Mark Robert Miller
        http://svn.apache.org/viewvc?view=revision&revision=1420336

        SOLR-4030: Allow rate limiting Directory IO based on the IO context.

        Show
        Commit Tag Bot added a comment - [trunk commit] Mark Robert Miller http://svn.apache.org/viewvc?view=revision&revision=1420336 SOLR-4030 : Allow rate limiting Directory IO based on the IO context.
        Hide
        Commit Tag Bot added a comment -

        [branch_4x commit] Mark Robert Miller
        http://svn.apache.org/viewvc?view=revision&revision=1420340

        SOLR-4030: Allow rate limiting Directory IO based on the IO context.

        Show
        Commit Tag Bot added a comment - [branch_4x commit] Mark Robert Miller http://svn.apache.org/viewvc?view=revision&revision=1420340 SOLR-4030 : Allow rate limiting Directory IO based on the IO context.
        Hide
        Mark Miller added a comment -

        I've added a test and committed.

        Show
        Mark Miller added a comment - I've added a test and committed.
        Hide
        Commit Tag Bot added a comment -

        [branch_4x commit] Mark Robert Miller
        http://svn.apache.org/viewvc?view=revision&revision=1420341

        SOLR-4030: messed up import somehow

        Show
        Commit Tag Bot added a comment - [branch_4x commit] Mark Robert Miller http://svn.apache.org/viewvc?view=revision&revision=1420341 SOLR-4030 : messed up import somehow

          People

          • Assignee:
            Mark Miller
            Reporter:
            Lucene Developers
          • Votes:
            1 Vote for this issue
            Watchers:
            8 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development