Details

    • Type: Bug
    • Status: Open
    • Priority: Minor
    • Resolution: Unresolved
    • Affects Version/s: 1.5.0, 1.6.0
    • Fix Version/s: 1.8.2, 2.0.0
    • Component/s: monitor
    • Labels:

      Description

      The monitor XML and JSON servlets differ greatly in what data they contain. The data should be gathered into JAXB POJOs in both servlets, and just serialzed differently so we're guaranteed to get the same data in both.

        Activity

        Hide
        jklucar Jim Klucar added a comment -

        No tests yet, but code seems to produce identical XML to previous Servlet, and identical JSON data.

        Show
        jklucar Jim Klucar added a comment - No tests yet, but code seems to produce identical XML to previous Servlet, and identical JSON data.
        Hide
        kturner Keith Turner added a comment -

        Why does the patch comment out com.google.code.gson in pom? Should it just be removed?

        I am not familiar with JAXB. It seems like it can serialize the new POJOs you created XML or JSON. I do not see in the patch where these new POJOs are populated?

        Show
        kturner Keith Turner added a comment - Why does the patch comment out com.google.code.gson in pom? Should it just be removed? I am not familiar with JAXB. It seems like it can serialize the new POJOs you created XML or JSON. I do not see in the patch where these new POJOs are populated?
        Hide
        jklucar Jim Klucar added a comment -

        Gson. Yes it should be removed. I commented it out and built to see for
        sure if anything other than what I changed used it, and I just never got
        back to removing it.

        All the monitor info is gathered in the Stats object constructor. That
        constructor recurs through all the other data objects, passing in various
        bits of the MasterMonitorInfo.

        My plan was to create another patch. This one is based on a github pull,
        not the new Accumulo git repo. I'll fix it up and re-submit a patch, and
        include a test case or two.

        Show
        jklucar Jim Klucar added a comment - Gson. Yes it should be removed. I commented it out and built to see for sure if anything other than what I changed used it, and I just never got back to removing it. All the monitor info is gathered in the Stats object constructor. That constructor recurs through all the other data objects, passing in various bits of the MasterMonitorInfo. My plan was to create another patch. This one is based on a github pull, not the new Accumulo git repo. I'll fix it up and re-submit a patch, and include a test case or two.
        Hide
        kturner Keith Turner added a comment -

        Jim Klucar answered my question, the constructor in o.a.a.server.monitor.servlets.jaxb.Stats() populates the POJOs.

        This patch is nice, it really cleans up the monitor code. As far as test, that would be excellent. Howerver the monitor does not currently have a lot of test, so I am thinking it should not be a requirement for this patch? Test for things that can easily be verified would great. For example if you know a table A has 10 tablets and table B has 3 tablets, does the XML accurately reflect this. Some things like # running compactions would difficult to verify in an intergration tests, maybe easier in a unit test that does not excecise full functionality. Jim Klucar if you do plan to implement test, let me know and I will hold off looking into this patch more. If there are no test I would like to closely review the code that populates the POJOs before applying it.

        Show
        kturner Keith Turner added a comment - Jim Klucar answered my question, the constructor in o.a.a.server.monitor.servlets.jaxb.Stats() populates the POJOs. This patch is nice, it really cleans up the monitor code. As far as test, that would be excellent. Howerver the monitor does not currently have a lot of test, so I am thinking it should not be a requirement for this patch? Test for things that can easily be verified would great. For example if you know a table A has 10 tablets and table B has 3 tablets, does the XML accurately reflect this. Some things like # running compactions would difficult to verify in an intergration tests, maybe easier in a unit test that does not excecise full functionality. Jim Klucar if you do plan to implement test, let me know and I will hold off looking into this patch more. If there are no test I would like to closely review the code that populates the POJOs before applying it.
        Hide
        vines John Vines added a comment -

        Given the lack of tests, I'm okay with this patch missing them. Jim Klucar can you make sure the patch is up to date so it can be applied?

        Show
        vines John Vines added a comment - Given the lack of tests, I'm okay with this patch missing them. Jim Klucar can you make sure the patch is up to date so it can be applied?
        Hide
        vines John Vines added a comment -

        Went ahead and tried to apply it. XMLServlet needed to be manually resolved, which is fine. However, the json page fails with java.lang.NoClassDefFoundError: org/codehaus/jettison/mapped/Configuration

        Show
        vines John Vines added a comment - Went ahead and tried to apply it. XMLServlet needed to be manually resolved, which is fine. However, the json page fails with java.lang.NoClassDefFoundError: org/codehaus/jettison/mapped/Configuration
        Hide
        vines John Vines added a comment -

        Bumping this to 1.7 unless Jim can get a functional patch in

        Show
        vines John Vines added a comment - Bumping this to 1.7 unless Jim can get a functional patch in

          People

          • Assignee:
            jklucar Jim Klucar
            Reporter:
            jklucar Jim Klucar
          • Votes:
            0 Vote for this issue
            Watchers:
            4 Start watching this issue

            Dates

            • Created:
              Updated:

              Development