6 Replies Latest reply on Apr 23, 2014 3:10 PM by imckinle

    RTGov ElasticSearch support

    objectiser

      Hi Ivan

       

      Thanks for the great work on RTGOV-342. I've merged the pull request more or less as is, with some style adjustments to pass the checkstyle validation (which is now enforced).

       

      Thought I would list some observations that we can discuss and use as the basis for some potential changes before the next release:

       

      1) I think we should remove the batch keystore implementation, until we have a better understanding of what needs to be achieved - especially as it is not currently implemented anyway. May be better just to offer additional configuration properties on the base implementation rather than two separate implementations.

       

      2) Hosts should be retrieved from RTGovProperties so that can be independently configured by administrator, rather than having to redeploy EPN if a change is required. If not defined, then use default value. Potentially the installer can be updated to ask if ES is used, and install epns and optional hosts configuration.

       

      3) Implementing the ElasticSearch functionality in the KeyStore service was a great idea. However, wondering whether an EventProcessor implementation specifically for ElasticSearch is now required? Instead, should it be made more generic (e.g. KeyValueStoreEventProcessor), as the ElasticSearch specifics are in the key store impl? Means that potentially other KeyStore impls (such as Cassandra, MongoDB, etc) could also be used with the same event processor?

       

      4) As mentioned in an email, I would like to have an "out of the box" EPN for use with ElasticSearch, to complement the integration of Kibana (and a default dashboard) that will be provided with the new RTGov UI. I think the current quickstart could provide the ideal default implementation, i.e. move it to the $rtgov/content folder, named elasticsearch-epn? Other quickstarts could then be provided to demonstrate other capabilities of the ES integration as required - but initially this "out of the box" epn could act as an example.

       

      5) Comments on the quickstart itself:

      - random mvel id script is not necessary - if no script is provided, then just call 'getRandom()' method on the KeyValueStore impl (which is not currently used), so script (and mvel) is only used if a custom approach for generating ids is required

      - quickstart predicates not required as these are the types published on the inbound subjects

      - no 'destinationSubjects' are required, as these event processors are not producing any results to be consumed by other EPNs

       

      6) Minor point - there are a number of 'todo' comments in the code. Could you record these as jiras instead, as it makes it easier to see what needs to be done.

       

       

      Once we have come to an agreement on these points, we can create additional jiras.

       

      Thanks again.

        • 1. Re: RTGov ElasticSearch support
          imckinle

          1) I think we should remove the batch keystore implementation, until we have a better understanding of what needs to be achieved - especially as it is not currently implemented anyway. May be better just to offer additional configuration properties on the base implementation rather than two separate implementations.

          > I agree,  I create a jira to address this. The jira will remove the bulk class. https://issues.jboss.org/browse/RTGOV-417

           

           

          2) Hosts should be retrieved from RTGovProperties so that can be independently configured by administrator, rather than having to redeploy EPN if a change is required. If not defined, then use default value. Potentially the installer can be updated to ask if ES is used, and install epns and optional hosts configuration.

          >  I’m not sure of this. it may be desired to have certain Stores to connect directly to a specific set of nodes as we know data is stored there.  Other wise ES will lookup where the data resides. Further investigated is need into how elastic searches clustering works before a anything is done here. 

           

           

          3) Implementing the ElasticSearch functionality in the KeyStore service was a great idea. However, wondering whether an EventProcessor implementation specifically for ElasticSearch is now required? Instead, should it be made more generic (e.g. KeyValueStoreEventProcessor), as the ElasticSearch specifics are in the key store impl? Means that potentially other KeyStore impls (such as Cassandra, MongoDB, etc) could also be used with the same event processor

          > Agree, it might be better to gentrify this functionality. Although it could be possible that certain key value store event processors may need to enrich or transform the data depending on implementation.  Eitherway. it would make sense to rename this  KeyValueStoreEventProcessor and should Store implementation specific(MongoDB, Cassandra) changes be required then we could just extend  from KeyValueStoreEventProcessor

           

          4) As mentioned in an email, I would like to have an "out of the box" EPN for use with ElasticSearch, to complement the integration of Kibana (and a default dashboard) that will be provided with the new RTGov UI. I think the current quickstart could provide the ideal default implementation, i.e. move it to the $rtgov/content folder, named elasticsearch-epn? Other quickstarts could then be provided to demonstrate other capabilities of the ES integration as required - but initially this "out of the box" epn could act as an example

          • Ok, Lets move it.

          5) Comments on the quickstart itself:

          - random mvel id script is not necessary - if no script is provided, then just call 'getRandom()' method on the KeyValueStore impl (which is not currently used), so script (and mvel) is only used if a custom approach for generating ids is required

          - quickstart predicates not required as these are the types published on the inbound subjects

          - no 'destinationSubjects' are required, as these event processors are not producing any results to be consumed by other EPNs

          > These changes can be made with point 5, “out of the box” EPN 

          6) Minor point - there are a number of 'todo' comments in the code. Could you record these as jiras instead, as it makes it easier to see what needs to be done.

          • Will do
          • 2. Re: RTGov ElasticSearch support
            imckinle

            Hey Gary,

             

            I have a suggestion.

            Enable OOTB elasticsearch for RTGov. in RTGov's current form users must have  ES downloaded and installed before they can begin using RTgov functionality. I feel this adds a(minor) unnecessary extra step.

            1) disable the Elasticsearch epn by default. This would require an extra flag

            2) much like we use hypersonic for OOTB. we could start up our own embedded ES node by default. this would also require a additional config flag

            • 3. Re: RTGov ElasticSearch support
              objectiser

              Hi Ivan

               

              I like the idea of (2) (see [RTGOV-346] Explore whether ElasticSearch server can be integrated into EAP - JBoss Issue Tracker).

               

              Would prefer it if it can be achieved without creating an EAP specific subsystem, so that it could apply to any environment in which rtgov is installed. Would be nice if, rather than using a server url of localhost:9200, we could just set the value to 'internal' by default and it would automatically use the internal server.

              • 4. Re: RTGov ElasticSearch support
                imckinle

                additionally this "embedded" ES would also be required to run automated tests againsts.

                • 5. Re: RTGov ElasticSearch support
                  objectiser

                  +1 - are you going to have time to investigate options for this next release?

                  • 6. Re: RTGov ElasticSearch support
                    imckinle

                    I can find time for this

                    I'm already having to think about this due to my junit test for the ActivityStore.