Make ExplicitLAC persistent

[jvrao]

FEATURE REQUEST

ExplicitLAC is kept in the memory and it can be lost in bookie reboot.
Though it is an extreme corner case scenario, it can break one of the BK guarantees.
" If you read once you can always read it".
If all the bookies of the Write stripe were rebooted, it can loose its explicitLAC value and the client
which was able to read the entry before the reboot, can't read it anymore.

[eolivelli]

This will be great.
I hope you already have some idea to share

[reddycharan]

@jvrao @sijie @eolivelli

For persisting explicitLAC, we can follow the same approach as followed in persisting fencing information / stateBits (d69986c) in FileInfo file and special marker entry in Journal.

We can have a new marker METAENTRY_ID_LEDGER_EXPLICITLAC = -0x8000 and we can log a special entry in journal (just like fence entry) when we get setExplicitLAC call to the bookie. This special entry could be following

    static ByteBuf createExplicitLACEntry(long ledgerId, long entryId) {
        ByteBuf bb = Unpooled.buffer(8 + 8 + 8);
        bb.writeLong(ledgerId);
        bb.writeLong(METAENTRY_ID_LEDGER_EXPLICITLAC);
        bb.writeLong(entryId);
        return bb;
    }

The long explicitLAC value could be stored in the header of the FileInfo after 'stateBits' value, as there is sufficient space remaining in FileInfo allocated header (1024). For explicitLAC, we can follow the same semantics as fence/statebits logic for flushHeader/checkpoint and replay journal .

But there is some small intricacy here regarding 'HEADER_VERSION' of the FileInfo. Since zero is valid ExplicitLac value for a Ledger, for backward compatibility purpose we cannot just consider the read zero value from HEADER after 'statebits' in old FileInfo file, as persisted 'ExplicitLAC'. So we may need to bump up 'HEADER_VERSION' of FileInfo for implementing this feature and handle compatibility scenarios.

[jvrao]

Yeah; we need to bump up the version number, and need to handle the case for backward compatibility.

[eolivelli]

Some thoughts (thinking out loud):

• I think we should unify ExplicitLAC and PiggyBackLAC, for instance we should update this new field on FileInfo both on setExplicitLAC and on addEntry
• We should change readLastAddConfirmed so that the reader will use this new value, and there will be no need to call readExplicitLAC, which is very confusing for the user
• If this setExplicitLAC operation is logged to the journal and we wait for fsync, it can be a good to update writer-side LAC in case of DEFERRED_SYNC

[sijie]

I think we should unify ExplicitLAC and PiggyBackLAC, for instance we should update this new field on FileInfo both on setExplicitLAC and on addEntry

We should change readLastAddConfirmed so that the reader will use this new value, and there will be no need to call readExplicitLAC, which is very confusing for the user

I don't think we should update FileInfo on addEntry. lac is already stored (piggyback) at the entries, we shouldn't couple this with explicit LAC. from end-user perspective, you can merge the view for piggybacked lac and explicit lac and provide one way to read it; but I don't think we should merge how we store it. it makes things complicated.

[eolivelli]

I don't think we should update FileInfo on addEntry.

Maybe we could save resources and do not write LAC on each entry. I don't know the original motivation of storing LAC for each entry. I thought it was only just to store data from network 'as-is' directly to the storage. @sijie you know the full history.
Anyway I am fine with keeping the two values separated at storage level.

To me it is really important that we have only one API for the read to access LAC, so I would like to drop readExplicitLAC

[sijie]

as I said, from end-user perspective it is good to have one API, however I don't see the need to merge these two different formats together.

[reddycharan]

Actually I missed a crucial aspect of explicitLAC in FileInfo. In FileInfo class, explicitLAC value is stored as ByteBuf but not simple long value. This ByteBuf contains Checksum/Digest information (variable length digestcode) as well, which would be needed to verify digest when explicitLAC is read back by the client. So to persist explicitLAC we received, we have to persist complete ByteBuf we received but not just long value.

So the special entry in Journal should be -

    static ByteBuf createExplicitLACEntry(long ledgerId, ByteBuf explicitLac) {
        ByteBuf bb = Unpooled.buffer(8 + 8 + 4 + explicitLac.capacity());
        bb.writeLong(ledgerId);
        bb.writeLong(METAENTRY_ID_LEDGER_EXPLICITLAC);
        bb.writeInt(explicitLac.capacity());
        bb.writeBytes(explicitLac);
        return bb;
    }

In FileInfo file to persist this variable length explicitLac in the header of FileInfo, we should write length of the explicitLac byteBuf first and then the actual explicitLac ByteBuf (just like how masterKey is persisted in the header of FileInfo). This length of explicitLac ByteBuf and actual ByteBuf should go to the Head of the FileInfo after statebits (this and need for changing version of HEADER is as described earlier and remains same).