Apache9 commented on PR #5016:
URL: https://github.com/apache/hbase/pull/5016#issuecomment-1423552347

   > > > This seems like an ingenious idea. But I want to confirm that due to 
the eviction mechanism of LRUMap, even if findEntry is used instead of 
addEntry, is there still a possibility of inconsistent read-write path behavior 
in theory?
   > > 
   > > 
   > > The most important thing here is to read WAL entries in order, and not 
skip any entries. If these two rules are guaranteed, it is OK to restart as 
many times as you want. And I think for replication, we must follow these two 
rules otherwise there will be data loss...
   > 
   > Agree about that, but I think I didn't express my question clearly.
   > 
   > For WAL Compression, The core logic is to build an index (LRUMap) in 
memory while writing/reading WAL. There is another key point here, that is, 
when operating a WAL file, the behavior of both read/write path needs to be 
exactly same.
   > 
   > Using findEntry instead of addEntry in this patch, I think it could solve 
a part of problem. But however, for example, we did not resetPosition when we 
wrote WAL, but a certain position was reset many times when we read WAL. The 
implicit operation here is: this node has been movedToHead many times in 
LRUMap. So is it possible that the node evicted in the write path(write WAL) 
has inconsistencies in the read path(replication)?
   
   After deep consideration I think you are right. The solution here can only 
work perfectly when the dict is infinite, i.e, no eviction. If we also consider 
eviction, if go back for a long distance, the word of a given index will change 
due to eviction, then when reading, if we use a index to get the word(a 
qualifier, a row, for example), we may get a incorrect word on the given index.
   In real world, although it is not likely that we will go back for a very 
long distance, but it is possible that a single WAL entry has a lot of cells, 
and our qualifier dict capacity is only 127, it is still possible to fall into 
the above scenario...
   
   So, it seems that rebuilding the dict is necessary when reseting. But 
anyway, I could try to refactor the readNext method in ProtrobufLogReader, to 
have more fine-grained control on whether we need to reconstruct the dict. For 
example, if we just return before reading the actual WAL entry, i.e, we quit 
earlier after checking available bytes, we do not need to reconstruct the dict.
   
   Thanks for pointing this out!


-- 
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.

To unsubscribe, e-mail: [email protected]

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

Reply via email to