lakshmi-manasa-g commented on pull request #1362: URL: https://github.com/apache/samza/pull/1362#issuecomment-631560831
@bkonold yes, your understanding is correct. Repair an invalid blob - yes it is possible to repair atleast partially using higher versions of Avro (i believe 1.9 onwards). Since we have not encountered such a blob I cant say for sure how much of the blob can be recovered. For the role of the last few bytes (aka synchronization marker) flushed by DFW - here is the documentation from DFW itself > Data written by DFW is grouped into blocks (not to confuse with Azure blocks uploaded). A synchronization marker is written between blocks, so that files may be split. DFW.close calls DFW.flush which inturn calls DFW.sync() whose doc says > Forces the end of the current block, emitting a synchronization marker. By default, this will also flush the block to the stream (AzureBlobOutputStream = ABOS in our case). I just realized when responding to your comment that AvroWriter.close is always preceeded by AvroWriter.flush - that is the requirement for close as per the documentation of AvroWriter and that is how it is used in the SystemProducer right now. Now, AvroWriter.flush also calls DFW.flush. Hence DFW.flush is called twice in succession and hence the second sync() becomes a no-op as there are no DFW blocks to force end of. This could very well be the reason why we have not seen invalid blobs due to this issue. Let me mull over this a bit more and get back. Part of me still says this PR is necessary for 2 reasons: it is essential to discard the blob and to not rely on DFW impl details (DFW.close calling flush or DFW.flush calling DFW.sync). And that since the exception will anyways be bubbled even if we commit the blob despite this exception - System producer user would most likely have re-tried the messages for this blob resulting in an another blob with same messages getting created. Two blobs instead of one does not break our promise but maybe undesirable. and another part of me says that this PR is not a must-have but a nice to-have: current impl of DFW and AvroWriter are working well enough to ensure that DFW.flush inside DFW.close is kind of no-op. So essentially DFW.close is doing only ABOS.close i think. So, in case of failure of DFW.close doing ABOS.close might not be totally bad. However, this is very dependent on the current impl of DFW and the binaryEncoders it uses to write to ABOS. ---------------------------------------------------------------- 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]
