ctubbsii commented on a change in pull request #1840:
URL: https://github.com/apache/accumulo/pull/1840#discussion_r545389383



##########
File path: 
server/tserver/src/main/java/org/apache/accumulo/tserver/scan/NextBatchTask.java
##########
@@ -89,14 +88,16 @@ public void run() {
       }
     } catch (TooManyFilesException | SampleNotPresentException e) {
       addResult(e);
-    } catch (OutOfMemoryError ome) {
-      Halt.halt("Ran out of memory scanning " + scanSession.extent + " for " + 
scanSession.client,
-          1);
-      addResult(ome);
-    } catch (Throwable e) {
+    } catch (Exception e) {
       log.warn("exception while scanning tablet "
           + (scanSession == null ? "(unknown)" : scanSession.extent), e);
       addResult(e);
+    } catch (Error t) {
+      log.warn(
+          "Error while scanning tablet " + (scanSession == null ? "(unknown)" 
: scanSession.extent),
+          t);

Review comment:
       > I'm not sure what yo umean, this is an copy of the log message in the 
catch clause above it.
   
   The previous message from the Halt had `" for " + scanSession.client`, but 
this log message does not mention the `scanSession.client` at all.
   
   > Yes, I mentioned that in the initial comment on the PR. These changes 
assume that #1818 gets merged.
   
   Thanks, I must've missed that.




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
[email protected]


Reply via email to