Details

    • Type: Sub-task Sub-task
    • Status: Resolved
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: None
    • Fix Version/s: 1.6.1, 1.7.0
    • Component/s: master, tserver
    • Labels:
      None

      Description

      The deprecation of ServerConfiguration causes two problems:

      1) eclipse is showing me ~300 warnings, so I'm missing other warnings in the noise

      2) ServerConfiguration is passed to some plug-ins (Balancer, at least), so any existing plug-ins are broken by the change.

      In the past, I would have been fine with just switching to the new class ServerConfigurationFactory but a growing user base and petabytes of existing systems gives me pause.

      Instead, I'm thinking we hoist the most popular methods to an abstract ServerConfiguration, and provide ServerConfiguationFactory as the implementation. This way the extensions continue to work.

      I'm still not sure what to do about static calls to get the SiteConfiguration.

        Issue Links

          Activity

          Eric Newton created issue -
          Bill Havanki made changes -
          Field Original Value New Value
          Link This issue is broken by ACCUMULO-2615 [ ACCUMULO-2615 ]
          Hide
          Bill Havanki added a comment -

          +1 on undeprecating ServerConfiguration. Sorry about the warnings.

          IMO a "ServerConfiguration" object should be some sort of configuration object, so I'm not in favor of having the factory implement it.

          I'm happy to work with you on figuring out some solution for the plugins. ServerConfiguration isn't part of the public API, though, is it? To what extent can we practically adjust it?

          I'm working on the static SiteConfiguration calls under ACCUMULO-2603. A complicating factor is that SiteConfiguration sort of belongs in o.a.a.server.conf and not the core, as Christopher Tubbs explained in ACCUMULO-2992, but I'm not planning to address that in the near term.

          Show
          Bill Havanki added a comment - +1 on undeprecating ServerConfiguration . Sorry about the warnings. IMO a "ServerConfiguration" object should be some sort of configuration object, so I'm not in favor of having the factory implement it. I'm happy to work with you on figuring out some solution for the plugins. ServerConfiguration isn't part of the public API, though, is it? To what extent can we practically adjust it? I'm working on the static SiteConfiguration calls under ACCUMULO-2603 . A complicating factor is that SiteConfiguration sort of belongs in o.a.a.server.conf and not the core, as Christopher Tubbs explained in ACCUMULO-2992 , but I'm not planning to address that in the near term.
          Hide
          Eric Newton added a comment -

          ServerConfiguration isn't officially in the public API, but since it is passed to a couple of plug-ins, it basically is. Sure, we can say "oh, that's not in the public API", but the developers with 1000's of lines of code are not going to be happy when their custom load balancer fails to run. Also, we're becoming part of a much larger ecosystem, and forcing change on users is getting less desirable.

          I agree that the name ServerConfiguration is unfortunate, and I don't mind deprecating it. I just want to preserve backwards compatibility for extensions for as long as possible.

          We can overload the methods that take ServerConfiguration and deprecate those.

          Show
          Eric Newton added a comment - ServerConfiguration isn't officially in the public API, but since it is passed to a couple of plug-ins, it basically is. Sure, we can say "oh, that's not in the public API", but the developers with 1000's of lines of code are not going to be happy when their custom load balancer fails to run. Also, we're becoming part of a much larger ecosystem, and forcing change on users is getting less desirable. I agree that the name ServerConfiguration is unfortunate, and I don't mind deprecating it. I just want to preserve backwards compatibility for extensions for as long as possible. We can overload the methods that take ServerConfiguration and deprecate those.
          Hide
          Christopher Tubbs added a comment -

          ServerConfiguration isn't the only non-public API item that is passed to pluggable components. We really need to make interfaces for our pluggable components, to make it easier to identify and implement (and to prevent leakage of non-public API into them). I'm hoping this can be done as part of ACCUMULO-2589 (or as follow-on actions, as we identify the pluggable components).

          We should avoid putting context-specific items into the API of pluggable components. For instance, instead of giving a balancer a ServerConfiguration parameter, we can give it a Configuration object called serverConf. That would separate the plugin implementation from the internal objects Accumulo uses, and the appropriate framework can decide how much or how little configuration to actually pass in the generic Configuration object (for instance, plugins shouldn't be getting shared secrets from the server configuration).

          Show
          Christopher Tubbs added a comment - ServerConfiguration isn't the only non-public API item that is passed to pluggable components. We really need to make interfaces for our pluggable components, to make it easier to identify and implement (and to prevent leakage of non-public API into them). I'm hoping this can be done as part of ACCUMULO-2589 (or as follow-on actions, as we identify the pluggable components). We should avoid putting context-specific items into the API of pluggable components. For instance, instead of giving a balancer a ServerConfiguration parameter, we can give it a Configuration object called serverConf. That would separate the plugin implementation from the internal objects Accumulo uses, and the appropriate framework can decide how much or how little configuration to actually pass in the generic Configuration object (for instance, plugins shouldn't be getting shared secrets from the server configuration).
          ASF subversion and git services logged work - 30/Jul/14 22:24
          ASF subversion and git services made changes -
          Remaining Estimate 0h [ 0 ]
          Time Spent 10m [ 600 ]
          Worklog Id 16782 [ 16782 ]
          Eric Newton made changes -
          Status Open [ 1 ] Resolved [ 5 ]
          Resolution Fixed [ 1 ]
          ASF subversion and git services logged work - 04/Aug/14 18:57
          ASF subversion and git services made changes -
          Time Spent 10m [ 600 ] 20m [ 1200 ]
          Worklog Id 16843 [ 16843 ]
          ASF subversion and git services logged work - 04/Aug/14 18:59
          ASF subversion and git services made changes -
          Time Spent 20m [ 1200 ] 0.5h [ 1800 ]
          Worklog Id 16846 [ 16846 ]
          Hide
          Christopher Tubbs added a comment -

          This issue still applies to the 1.6.1-SNAPSHOT branch.

          Show
          Christopher Tubbs added a comment - This issue still applies to the 1.6.1-SNAPSHOT branch.
          Christopher Tubbs made changes -
          Resolution Fixed [ 1 ]
          Status Resolved [ 5 ] Reopened [ 4 ]
          Christopher Tubbs made changes -
          Fix Version/s 1.6.1 [ 12325441 ]
          Hide
          Christopher Tubbs added a comment -

          Discussed with Eric Newton and it might make the most sense to simply remove the deprecation on the 1.6 branch, to get rid of the noise. The migration to the ServerConfigurationFactory is already in motion in the master branch, so I think that is the best route.

          Show
          Christopher Tubbs added a comment - Discussed with Eric Newton and it might make the most sense to simply remove the deprecation on the 1.6 branch, to get rid of the noise. The migration to the ServerConfigurationFactory is already in motion in the master branch, so I think that is the best route.
          ASF subversion and git services logged work - 22/Aug/14 23:06
          • Time Spent:
            10m
             
            Commit 705a60d19e55c3499c168d3dd3ad5215dd8487c4 in accumulo's branch refs/heads/1.6.1-SNAPSHOT from [~ctubbsii]
            [ https://git-wip-us.apache.org/repos/asf?p=accumulo.git;h=705a60d ]

            ACCUMULO-3019 "Un-deprecate" ServerConfiguration

              Suppress all the unnecessary warnings noise due to the deprecation of this
              class without removing existing usage. One cannot reasonably consider it
              deprecated if we are still using it so extensively. This only applies to the
              current branch. It is deprecated in future branches. This commit should be
              merged to newer branches with the "ours" merge strategy.
          ASF subversion and git services made changes -
          Time Spent 0.5h [ 1800 ] 40m [ 2400 ]
          Worklog Id 17256 [ 17256 ]
          ASF subversion and git services logged work - 22/Aug/14 23:06
          • Time Spent:
            10m
             
            Commit 705a60d19e55c3499c168d3dd3ad5215dd8487c4 in accumulo's branch refs/heads/master from [~ctubbsii]
            [ https://git-wip-us.apache.org/repos/asf?p=accumulo.git;h=705a60d ]

            ACCUMULO-3019 "Un-deprecate" ServerConfiguration

              Suppress all the unnecessary warnings noise due to the deprecation of this
              class without removing existing usage. One cannot reasonably consider it
              deprecated if we are still using it so extensively. This only applies to the
              current branch. It is deprecated in future branches. This commit should be
              merged to newer branches with the "ours" merge strategy.
          ASF subversion and git services made changes -
          Time Spent 40m [ 2400 ] 50m [ 3000 ]
          Worklog Id 17258 [ 17258 ]
          Christopher Tubbs made changes -
          Status Reopened [ 4 ] Resolved [ 5 ]
          Resolution Fixed [ 1 ]
          Transition Time In Source Status Execution Times Last Executer Last Execution Date
          Open Open Resolved Resolved
          5d 7h 50m 1 Eric Newton 30/Jul/14 22:28
          Resolved Resolved Reopened Reopened
          11d 23h 35m 1 Christopher Tubbs 11/Aug/14 22:03
          Reopened Reopened Resolved Resolved
          11d 2h 4m 1 Christopher Tubbs 23/Aug/14 00:08

            People

            • Assignee:
              Eric Newton
              Reporter:
              Eric Newton
            • Votes:
              0 Vote for this issue
              Watchers:
              3 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Time Tracking

                Estimated:
                Original Estimate - Not Specified
                Not Specified
                Remaining:
                Remaining Estimate - 0h
                0h
                Logged:
                Time Spent - 50m
                50m

                  Development