[ 
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)

Reply via email to