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