Uploaded image for project: 'Chukwa'
  1. Chukwa
  2. CHUKWA-268

Create an interface to manage all Adaptors

    Details

    • Type: Improvement
    • Status: Resolved
    • Priority: Major
    • Resolution: Fixed
    • Affects Version/s: 0.2.0
    • Fix Version/s: 0.2.0
    • Component/s: Data Collection
    • Labels:
      None

      Description

      Currently the agent manage all adaptors and it's fine however it will be cleaner to have an AdaptorManager interface. The agent will just implement this interface without changing anything, but adaptors will retrieve the manager using ChukwaFactory. This will remove the hard dependency between Agent and adaptors.

      1. CHUKWA-268.patch
        43 kB
        Ari Rabkin
      2. CHUKWA-268.patch
        38 kB
        Ari Rabkin

        Issue Links

          Activity

          Hide
          asrabkin Ari Rabkin added a comment -

          I would make AdaptorManager an interface that's implemented by ChukwaAgent, and would pass it in to the adaptor at initialization time. This AdaptorManager can subsume ChunkReceiver.

          Show
          asrabkin Ari Rabkin added a comment - I would make AdaptorManager an interface that's implemented by ChukwaAgent, and would pass it in to the adaptor at initialization time. This AdaptorManager can subsume ChunkReceiver.
          Hide
          jboulon Jerome Boulon added a comment -

          Ari,
          Why this issue blocked CHUKWA-265 & CHUKWA-204?
          Also, this is based on a previous discussion we had a long time ago but I couldn't found the Jira where we talked about that, any idea?

          Show
          jboulon Jerome Boulon added a comment - Ari, Why this issue blocked CHUKWA-265 & CHUKWA-204 ? Also, this is based on a previous discussion we had a long time ago but I couldn't found the Jira where we talked about that, any idea?
          Hide
          asrabkin Ari Rabkin added a comment - - edited

          This issue blocks CHUKWA-265 and CHUKWA-204 because patches for those Jiras should almost certainly use this API. I also remember discussing this before – but I couldn't find a Jira for it either.

          (Though since CHUKWA-265 is a blocker, we might want to do something slightly more ad-hoc to resolve it now, and then revise to use the new API)

          Show
          asrabkin Ari Rabkin added a comment - - edited This issue blocks CHUKWA-265 and CHUKWA-204 because patches for those Jiras should almost certainly use this API. I also remember discussing this before – but I couldn't find a Jira for it either. (Though since CHUKWA-265 is a blocker, we might want to do something slightly more ad-hoc to resolve it now, and then revise to use the new API)
          Hide
          jboulon Jerome Boulon added a comment -

          I'm still unable to reproduce CHUKWA-265 but assuming there's a bug I would like to have a patch asap then this Jira will take care of the migration.
          So can we remove the dependency for CHUKWA-265?

          Show
          jboulon Jerome Boulon added a comment - I'm still unable to reproduce CHUKWA-265 but assuming there's a bug I would like to have a patch asap then this Jira will take care of the migration. So can we remove the dependency for CHUKWA-265 ?
          Hide
          asrabkin Ari Rabkin added a comment -

          Dependency removed; now marked merely as "related"

          Show
          asrabkin Ari Rabkin added a comment - Dependency removed; now marked merely as "related"
          Hide
          asrabkin Ari Rabkin added a comment -

          Create adaptor management interface, remove most access to agent internals, create AbstractAdaptor class (as per mailing list discussion).

          Show
          asrabkin Ari Rabkin added a comment - Create adaptor management interface, remove most access to agent internals, create AbstractAdaptor class (as per mailing list discussion).
          Hide
          eyang Eric Yang added a comment -

          processAddCommand("add org.apache.hadoop.chukwa.datacollection.adaptor.ExecAdaptor ps ps aux 0");

          The processCommand used to support multiple type of commands. If the interface is changing to processAddCommand, perhaps, we don't need add as part of the string.

          Could we refine the adaptor interface to?

          stopAdaptor(param, GRACEFUL|FORCED)
          listAdaptor(param)
          startAdaptor(param)

          processCommand("add param");
          processCommand("list");
          processCommand("shutdown param");
          processCommand("stop param");

          Show
          eyang Eric Yang added a comment - processAddCommand("add org.apache.hadoop.chukwa.datacollection.adaptor.ExecAdaptor ps ps aux 0"); The processCommand used to support multiple type of commands. If the interface is changing to processAddCommand, perhaps, we don't need add as part of the string. Could we refine the adaptor interface to? stopAdaptor(param, GRACEFUL|FORCED) listAdaptor(param) startAdaptor(param) processCommand("add param"); processCommand("list"); processCommand("shutdown param"); processCommand("stop param");
          Hide
          asrabkin Ari Rabkin added a comment -

          Agent.processCommand() has never handled anything other than "add". All those other commands are implemented in AgentControlSocketListener. The renaming just reflects the status quo.

          We can certainly add more things to the agent control interface. I'm open to advice on whether adaptors should have access to the full generality of agent functionality, or only to a subset. I initially opted for subset, but mostly because it's easier to add things to an interface than to revoke them in the future.

          Show
          asrabkin Ari Rabkin added a comment - Agent.processCommand() has never handled anything other than "add". All those other commands are implemented in AgentControlSocketListener. The renaming just reflects the status quo. We can certainly add more things to the agent control interface. I'm open to advice on whether adaptors should have access to the full generality of agent functionality, or only to a subset. I initially opted for subset, but mostly because it's easier to add things to an interface than to revoke them in the future.
          Hide
          asrabkin Ari Rabkin added a comment -

          Revised as per discussion with Eric.
          -More cleanly support list() operation in control interface
          -Clarify processAddCommand() in code.

          Show
          asrabkin Ari Rabkin added a comment - Revised as per discussion with Eric. -More cleanly support list() operation in control interface -Clarify processAddCommand() in code.
          Hide
          asrabkin Ari Rabkin added a comment -

          Passes unit tests at my end.

          Show
          asrabkin Ari Rabkin added a comment - Passes unit tests at my end.
          Hide
          eyang Eric Yang added a comment -

          +1 Looks good.

          Show
          eyang Eric Yang added a comment - +1 Looks good.
          Hide
          asrabkin Ari Rabkin added a comment -

          I just committed this.

          Show
          asrabkin Ari Rabkin added a comment - I just committed this.
          Hide
          hudson Hudson added a comment -

          Integrated in Chukwa-trunk #49 (See http://hudson.zones.apache.org/hudson/job/Chukwa-trunk/49/)
          . Some missing files from prev. patch
          . Adaptor manager interface, Agent class refactoring.

          Show
          hudson Hudson added a comment - Integrated in Chukwa-trunk #49 (See http://hudson.zones.apache.org/hudson/job/Chukwa-trunk/49/ ) . Some missing files from prev. patch . Adaptor manager interface, Agent class refactoring.

            People

            • Assignee:
              asrabkin Ari Rabkin
              Reporter:
              jboulon Jerome Boulon
            • Votes:
              0 Vote for this issue
              Watchers:
              0 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Development