comnetwork edited a comment on pull request #1970:
URL: https://github.com/apache/hbase/pull/1970#issuecomment-649947973


   
   
   > > > That is to say, AsyncFSWAL.getLogFileSizeIfBeingWritten could not 
reflect the file length which successfully synced to underlying HDFS, which is 
not as expected.
   > > 
   > > 
   > > Wasn't that intentional, as a mean to proper track WAL files still open 
for write? For example, in case of replication, it should go as far as any 
entry got already appended, no? Ping @Apache9 who worked on this before to give 
more thoughts.
   > 
   > This guy contacted me offline and I confirmed that this should be a 
problem.
   > 
   > What I can recall is that, when doing some bug fixes and improving the 
performance in AsyncFSWAL, I changed the way we calculate the length of the 
writer. Maybe I forget the assumption in HBASE-14004 when doing these changes 
and lead to the problem.
   > 
   > So @comnetwork , please add more comments to say why we need the 
getSyncedLength method in the WAL.Writer interface? So later people will not 
break it again.
   > 
   > Thanks.
   
   @Apache9 , I already added comments for` WriteBase.getSyncedLength` like 
following:
   
    /**
     * NOTE: We add this method for {@link WALFileLengthProvider} used for 
replication, considering the
     * case if we use {@link AsyncFSWAL},we write to 3 DNs 
concurrently,according to the visibility
     * guarantee of HDFS, the data will be available immediately when arriving 
at DN since all the DNs
     * will be considered as the last one in pipeline. This means replication 
may read uncommitted data
     * and replicate it to the remote cluster and cause data inconsistency.
     * The method {@link WriterBase#getLength} may return length which just in 
hdfs client buffer and not
     * successfully synced to HDFS, so we use this method to return the length 
successfully synced to HDFS
     * and replication thread could only read writing WAL file limited by this 
length.
     * see also HBASE-14004 and this document for more details:
     * 
https://docs.google.com/document/d/11AyWtGhItQs6vsLRIx32PwTxmBY3libXwGXI25obVEY/edit#
     */


----------------------------------------------------------------
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:
us...@infra.apache.org


Reply via email to