Ian Maxon has posted comments on this change.

Change subject: [NO ISSUE] Report all BufferCache write failures.
......................................................................


Patch Set 5:

(4 comments)

https://asterix-gerrit.ics.uci.edu/#/c/2787/5/hyracks-fullstack/hyracks/hyracks-storage-am-bloomfilter/src/main/java/org/apache/hyracks/storage/am/bloomfilter/impls/BloomFilter.java
File 
hyracks-fullstack/hyracks/hyracks-storage-am-bloomfilter/src/main/java/org/apache/hyracks/storage/am/bloomfilter/impls/BloomFilter.java:

PS5, Line 288:       private volatile Throwable failure = null;
how can 2 threads access this?


https://asterix-gerrit.ics.uci.edu/#/c/2787/5/hyracks-fullstack/hyracks/hyracks-storage-am-lsm-invertedindex/src/main/java/org/apache/hyracks/storage/am/lsm/invertedindex/impls/LSMInvertedIndexDiskComponent.java
File 
hyracks-fullstack/hyracks/hyracks-storage-am-lsm-invertedindex/src/main/java/org/apache/hyracks/storage/am/lsm/invertedindex/impls/LSMInvertedIndexDiskComponent.java:

PS5, Line 116: if (callback.hasFailed()) {
             :             throw 
HyracksDataException.create(callback.getFailure());
             :         }
why not else?..


https://asterix-gerrit.ics.uci.edu/#/c/2787/5/hyracks-fullstack/hyracks/hyracks-storage-common/src/main/java/org/apache/hyracks/storage/common/buffercache/AsyncFIFOPageQueueManager.java
File 
hyracks-fullstack/hyracks/hyracks-storage-common/src/main/java/org/apache/hyracks/storage/common/buffercache/AsyncFIFOPageQueueManager.java:

PS5, Line 72:   LOGGER.error("An attempt to write a page found buffer cache 
closed");
            :                     
ExitUtil.halt(ExitUtil.EC_ABNORMAL_TERMINATION);
            :                 }
            :             } catch (InterruptedException e) {
            :                 LOGGER.error("IO Operation interrupted", e);
            :                 ExitUtil.halt(ExitUtil.EC_ABNORMAL_TERMINATION);
this condition will probably happen if we ask for shutdown while a write is 
ongoing, is it ok to invoke exitutil like this in that scenario?


https://asterix-gerrit.ics.uci.edu/#/c/2787/5/hyracks-fullstack/hyracks/hyracks-storage-common/src/main/java/org/apache/hyracks/storage/common/buffercache/IFIFOPageQueue.java
File 
hyracks-fullstack/hyracks/hyracks-storage-common/src/main/java/org/apache/hyracks/storage/common/buffercache/IFIFOPageQueue.java:

PS5, Line 18: // Isn't a queue FIFO by default? rename to IPageWriteQueue?
> I don't think all I/O queues are FIFO by default. For example, the OS can r
the fifo in this just comes from how we were calling it at the time, in 
contrast to how the buffercache flushes pages (clock). as in there would be two 
write paths in general, fifo and normal. but now we're only using fifo.


-- 
To view, visit https://asterix-gerrit.ics.uci.edu/2787
To unsubscribe, visit https://asterix-gerrit.ics.uci.edu/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I97fd3dccff85dab84d644359be6f66b15ee708ef
Gerrit-PatchSet: 5
Gerrit-Project: asterixdb
Gerrit-Branch: master
Gerrit-Owner: abdullah alamoudi <[email protected]>
Gerrit-Reviewer: Anon. E. Moose #1000171
Gerrit-Reviewer: Ian Maxon <[email protected]>
Gerrit-Reviewer: Jenkins <[email protected]>
Gerrit-Reviewer: Luo Chen <[email protected]>
Gerrit-Reviewer: Michael Blow <[email protected]>
Gerrit-Reviewer: Murtadha Hubail <[email protected]>
Gerrit-Reviewer: Till Westmann <[email protected]>
Gerrit-HasComments: Yes

Reply via email to