Uploaded image for project: 'Cassandra'
  1. Cassandra
  2. CASSANDRA-5045

Add Configuration Loader to DatabaseDescriptor

    Details

    • Type: New Feature
    • Status: Resolved
    • Priority: Minor
    • Resolution: Fixed
    • Fix Version/s: 2.0 beta 1
    • Component/s: None
    • Labels:
      None

      Description

      Based on feed back from CASSANDRA-4998 -

      Added IConfigurationLoader that is used to load an external Config. When the System Property cassandra.config.loader is defined with an implementing class, DatabaseDescriptor with will that Config over loading the cassandra.yaml.

      1. 0001-Add-configuration-loader-to-DatabaseDescriptor.patch
        48 kB
        Michael Guymon
      2. 5045_v3.txt
        49 kB
        Sylvain Lebresne
      3. 5045-v2.txt
        48 kB
        Sylvain Lebresne

        Issue Links

          Activity

          Hide
          mguymon Michael Guymon added a comment -

          I took a stab at asserting the logic used in DatabaseDesciptor to produce the final Config. Unfortunately, during the processing of the Config, when the DynamicSnitch and the MessagingService are created, they reference back to the DatabaseDescriptor's Config. This circular reference makes it difficult (impossible?) to build a Config to test without changing the one DatabaseDescriptor is using.

          Show
          mguymon Michael Guymon added a comment - I took a stab at asserting the logic used in DatabaseDesciptor to produce the final Config. Unfortunately, during the processing of the Config, when the DynamicSnitch and the MessagingService are created, they reference back to the DatabaseDescriptor's Config. This circular reference makes it difficult (impossible?) to build a Config to test without changing the one DatabaseDescriptor is using.
          Hide
          jbellis Jonathan Ellis added a comment -

          Should we wontfix CASSANDRA-4998 then?

          Show
          jbellis Jonathan Ellis added a comment - Should we wontfix CASSANDRA-4998 then?
          Hide
          slebresne Sylvain Lebresne added a comment -

          Looks fine but since we add a new ConfigurationLoader interface, we can just as well make the yaml loading just a special loader that is used by default and simplify things slightly. Attaching a v2 that does that.

          Should we wontfix CASSANDRA-4998 then?

          Yes, done.

          Show
          slebresne Sylvain Lebresne added a comment - Looks fine but since we add a new ConfigurationLoader interface, we can just as well make the yaml loading just a special loader that is used by default and simplify things slightly. Attaching a v2 that does that. Should we wontfix CASSANDRA-4998 then? Yes, done.
          Hide
          jbellis Jonathan Ellis added a comment -

          Michael, can you review v2?

          Show
          jbellis Jonathan Ellis added a comment - Michael, can you review v2?
          Hide
          mguymon Michael Guymon added a comment -

          Looks like while the ticket slumbered, DatabaseDescriptor evolved a little. The v2 patch does not correctly apply to the latest changes on trunk DatabaseDescriptor.

          The patch itself looks good to me, simple is better.

          Show
          mguymon Michael Guymon added a comment - Looks like while the ticket slumbered, DatabaseDescriptor evolved a little. The v2 patch does not correctly apply to the latest changes on trunk DatabaseDescriptor. The patch itself looks good to me, simple is better.
          Hide
          slebresne Sylvain Lebresne added a comment -

          Attached rebased v2 patch. I do note that the patch is against the cassandra-1.2 branch, not trunk.

          Show
          slebresne Sylvain Lebresne added a comment - Attached rebased v2 patch. I do note that the patch is against the cassandra-1.2 branch, not trunk.
          Hide
          jbellis Jonathan Ellis added a comment -

          Un-tagging from 1.2.1 since review keeps stalling.

          Show
          jbellis Jonathan Ellis added a comment - Un-tagging from 1.2.1 since review keeps stalling.
          Hide
          jasobrown Jason Brown added a comment - - edited

          Sylvain Lebresne, if you can rebase again, I'll review the patch.

          Show
          jasobrown Jason Brown added a comment - - edited Sylvain Lebresne , if you can rebase again, I'll review the patch.
          Hide
          slebresne Sylvain Lebresne added a comment -

          Rebased version attached as v3, against trunk now (since at the point I think the 1.2 ship has sailed).

          Show
          slebresne Sylvain Lebresne added a comment - Rebased version attached as v3, against trunk now (since at the point I think the 1.2 ship has sailed).
          Hide
          jasobrown Jason Brown added a comment -

          v3 LGTM.

          Show
          jasobrown Jason Brown added a comment - v3 LGTM.
          Hide
          slebresne Sylvain Lebresne added a comment -

          Committed, thanks

          Show
          slebresne Sylvain Lebresne added a comment - Committed, thanks

            People

            • Assignee:
              slebresne Sylvain Lebresne
              Reporter:
              mguymon Michael Guymon
              Reviewer:
              Sylvain Lebresne
            • Votes:
              0 Vote for this issue
              Watchers:
              3 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Development