[jira] [Comment Edited] (RATIS-477) Improve LogStateMachine.processReadRequest()

["Vladimir Rodionov (JIRA)" <ji...@apache.org>]

    [ https://issues.apache.org/jira/browse/RATIS-477?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16758598#comment-16758598 ] 

Vladimir Rodionov edited comment on RATIS-477 at 2/1/19 7:24 PM:
-----------------------------------------------------------------

I think that we have to avoid future technical debt from the very beginning. 

We provide read log entry by Id only for one use case - to resume scanning log entries from a last known position (replication use case). Some inefficiency in
locating this position is OK, as since this is going to be non frequent operation. Keeping in memory index for that? Not sure it is worth it. 

{code}
/**
 * Reads (potentially) multiple log entries starting from a given startId
 * @param buffer
 * @param startId
 * @return next logId to read next log entries from or 
       the same startId if available space in a buffer is not sufficient
 **/ 
long nextId  read(ByteBuffer buffer, long startId)
{code}
This method always reads at least one whole Raft log entry, it skips Ratis internal log entries and never stops reading in the middle
of a Raft log entry (if it is multi-log entry - all log entries MUST be read)
We can provide utility class which can extract log records blobs from a buffer, as well as number of records

{code}

long nextId = read(buffer, startId);
if (nextId != startId) {
  // successful read
int numRead = Utils.numEntries(buffer);
Iterator<byte[]> entries = Utils.iterator(buffer);
while(entries.hasNext()) {
   byte[] entry = entries.next();
}
{code}

This is raw access low level. The big plus - no need for index at all, the downside - some additional explanation is required fro users because, in general, nextId - startId != numRead. 


was (Author: vrodionov):
I think that we have to avoid future technical debt from the very beginning. 

We provide read log entry by Id only for one use case - to resume scanning log entries from a last known position (replication use case). Some inefficiency in
locating this position is OK, as since this is going to be non frequent operation. Keeping in memory index for that? Not sure it is worth it. 

{code}
/**
 * Reads (potentially) multiple log entries starting from a given startId
 * @param buffer
 * @param startId
 * @return next logId to read next log entries from or 
       the same startId if available space in a buffer is not sufficient
 **/ 
long nextId  read(ByteBuffer buffer, long startId)
{code}
This method always reads at least one whole Raft log entry, it skips Ratis internal log entries and never stops reading in the middle
of a Raft log entry (if it is multi-log entry - all log entries MUST be read)
We can provide utility class which can extract log records blobs from a buffer, as well as number of records

{code}

long nextId = read(buffer, startId);
if (nextId != startId) {
  // successful read
int numRead = Utils.numEntries(buffer);
Iterator<byte[]> entries = Utils.iterator(buffer);
while(entries.hasNext()) {
   byte[] entry = entries.next();
}
{code}

This is raw access low level. 

> Improve LogStateMachine.processReadRequest()
> --------------------------------------------
>
>                 Key: RATIS-477
>                 URL: https://issues.apache.org/jira/browse/RATIS-477
>             Project: Ratis
>          Issue Type: Improvement
>          Components: LogService
>            Reporter: Josh Elser
>            Assignee: Josh Elser
>            Priority: Major
>             Fix For: 0.4.0
>
>
> [~vrodionov] and [~rajeshbabu] were asking about this in RATIS-470. Can we improve the logic that reads through the "head" of the RaftLog to find the "LogService offset"?
> Vlad suggested:
> {quote}
> Josh, can we simplify this a little bit? Client gets Raft log ID one every append, for multi-get calls we can use this LogID (RAFT) as our start log ID to avoid iterating through all logs, then you can unwind log entries by using new logic, see above:
>  int numRecordsInAppend = append.getDataCount();
> There is no need to iterate from the very beginning
> {quote}



--
This message was sent by Atlassian JIRA
(v7.6.3#76005)