metron-issues mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "ASF GitHub Bot (JIRA)" <j...@apache.org>
Subject [jira] [Commented] (METRON-1771) Update REST endpoints to support eventually consistent UI updates
Date Thu, 04 Oct 2018 17:01:00 GMT

    [ https://issues.apache.org/jira/browse/METRON-1771?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16638537#comment-16638537
] 

ASF GitHub Bot commented on METRON-1771:
----------------------------------------

Github user nickwallen commented on a diff in the pull request:

    https://github.com/apache/metron/pull/1190#discussion_r222750671
  
    --- Diff: metron-platform/metron-indexing/src/main/java/org/apache/metron/indexing/dao/MultiIndexDao.java
---
    @@ -282,4 +276,27 @@ public Document getLatest(final String guid, String sensorType) throws
IOExcepti
       public List<IndexDao> getIndices() {
         return indices;
       }
    +
    +  private Document getDocument(List<DocumentContainer> documentContainers) throws
IOException {
    +    Document ret = null;
    +    List<String> error = new ArrayList<>();
    +    for(DocumentContainer dc : documentContainers) {
    +      if(dc.getException().isPresent()) {
    +        Throwable e = dc.getException().get();
    +        error.add(e.getMessage() + "\n" + ExceptionUtils.getStackTrace(e));
    +      }
    +      else {
    +        if(dc.getDocument().isPresent()) {
    +          Document d = dc.getDocument().get();
    +          if(ret == null || ret.getTimestamp() < d.getTimestamp()) {
    +            ret = d;
    +          }
    +        }
    +      }
    +    }
    --- End diff --
    
    > The question is what should happen when this unexpected condition does happen? Should
it blow up and throw and exception or return a document if at least one DAO returns a valid
one.
    
    Ah, OK.  I'm groking it now.  We want it to return null; at least based on the javadocs
defined in `RetrieveLatestDao.getLatest` and other places as null means nothing was found.
 So functionally, we don't want it to throw that exception.  
    
    But IMO it might benefit from some clarity.  I would suggest changing `getDocument` to
`getLatestDocument` with some commentary and clarity on what it means if there is no Document
or exception and what happens if there is an error in one of the underlying indices.
    
    Feel free to tweak to your liking or disregard, but this is what I would do.
    ```
      /**
       * Returns the most recent {@link Document} from a list of {@link DocumentContainer}s.
       *
       * @param documentContainers A list of containers; each retrieved from a separate index.
       * @return The latest {@link Document} found.
       * @throws IOException If any of the {@link DocumentContainer}s contain an exception.
       */
      private Document getLatestDocument(List<DocumentContainer> documentContainers)
throws IOException {
        Document latestDocument = null;
        List<String> error = new ArrayList<>();
    
        for(DocumentContainer dc : documentContainers) {
          if(dc.getException().isPresent()) {
            // collect each exception; multiple can occur, one in each index
            Throwable e = dc.getException().get();
            error.add(e.getMessage() + "\n" + ExceptionUtils.getStackTrace(e));
    
          } else if(dc.getDocument().isPresent()) {
            Document d = dc.getDocument().get();
            // is this the latest document so far?
            if(latestDocument == null || latestDocument.getTimestamp() < d.getTimestamp())
{
              latestDocument = d;
            }
    
          } else {
            // no document was found in the index
            latestDocument = null;
          }
        }
        if(error.size() > 0) {
          // report all of the errors encountered
          throw new IOException(Joiner.on("\n").join(error));
        }
        return latestDocument;
      }
    ```
    



> Update REST endpoints to support eventually consistent UI updates
> -----------------------------------------------------------------
>
>                 Key: METRON-1771
>                 URL: https://issues.apache.org/jira/browse/METRON-1771
>             Project: Metron
>          Issue Type: Improvement
>            Reporter: Ryan Merriman
>            Priority: Major
>
> Currently the REST endpoints that perform document updates either return true/false
or nothing.  This puts the responsibility of retrieving the updated state of the object on
the client in a separate call or optimistically applying the changes and reverting when an
update fails.  This can be problematic if a client attempts to get the current state immediately
after an update and the change isn't visible yet in the back end.
> Ideally they should return the updated state of the object, eliminating the need to
look up the updated state in a separate call.



--
This message was sent by Atlassian JIRA
(v7.6.3#76005)

Mime
View raw message