APEXMALHAR-2566 Fixed NPE in FSWindowDataManager #732
Conversation
|
Is it possible to come up with a unit test for this. Looks like the NPE happens only in some cases. |
|
There are checkstyle errors, please see [ERROR] src/main/java/org/apache/apex/malhar/lib/wal/FSWindowDataManager.java:[487,11] (whitespace) WhitespaceAround: WhitespaceAround: 'if' is not followed by whitespace. This looks like an edge case as this code generally works and doesn't error always. If it is an edge case and you are unable to come up with a unit test due to that reason, if there is no objection from others, I am ok to merge it without a unit test in this case. |
|
I am not fully convinced that this is the right fix without a unit test. Looking at |
| @@ -484,6 +484,10 @@ private Object retrieve(FSWindowReplayWAL wal, long windowId) throws IOException | |||
|
|
|||
| if (wal.retrievedWindow == null) { | |||
| wal.retrievedWindow = readNext(reader); | |||
| if(wal.retrievedWindow == null && | |||
| reader.getCurrentPointer().compareTo(wal.walEndPointerAfterRecovery) < 0) { | |||
There was a problem hiding this comment.
What if wal.retrievedWindow == null && reader.getCurrentPointer().compareTo(wal.walEndPointerAfterRecovery) >= 0?
| if(wal.retrievedWindow == null && | ||
| reader.getCurrentPointer().compareTo(wal.walEndPointerAfterRecovery) < 0) { | ||
| continue; | ||
| } | ||
| Preconditions.checkNotNull(wal.retrievedWindow); |
There was a problem hiding this comment.
There is an assumption that wal.retrievedWindow != null that does not seems to be valid.
|
@patilvikram can you respond to @vrozov comments |
No description provided.