wchevreuil commented on code in PR #5371:
URL: https://github.com/apache/hbase/pull/5371#discussion_r1321395417
##########
hbase-server/src/main/java/org/apache/hadoop/hbase/io/hfile/bucket/BucketCache.java:
##########
@@ -1700,17 +1853,17 @@ public BucketEntry writeToCache(final IOEngine
ioEngine, final BucketAllocator a
HFileBlock block = (HFileBlock) data;
ByteBuff sliceBuf = block.getBufferReadOnly();
block.getMetaData(metaBuff);
- ioEngine.write(sliceBuf, offset);
// adds the cache time after the block and metadata part
if (isCachePersistent) {
- ioEngine.write(metaBuff, offset + len - metaBuff.limit() -
Long.BYTES);
ByteBuffer buffer = ByteBuffer.allocate(Long.BYTES);
buffer.putLong(bucketEntry.getCachedTime());
buffer.rewind();
- ioEngine.write(buffer, (offset + len - Long.BYTES));
+ ioEngine.write(buffer, offset);
+ ioEngine.write(sliceBuf, offset + Long.BYTES);
Review Comment:
> I'm wondering we are changing the order of reads/writes, Are they backward
compatible if someone has a bucket cache already persisted during rolling
restarts?
Yeah, it's not backward compatible, the cache would be reset. This change is
for addressing the possibility of transaction file corruption currently
simulated by the newly added TestRecoveryPersistentBucketCache.
> And I see reads are updated on only FileIOEngine, don't we need to update
other Engines too?
FileIOEngine is the only one that supports "cache persistence".
--
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]
