Skip to content

clean up record logic#4

Open
kgioioso wants to merge 1 commit into
mainfrom
katie-record-cleanup
Open

clean up record logic#4
kgioioso wants to merge 1 commit into
mainfrom
katie-record-cleanup

Conversation

@kgioioso
Copy link
Copy Markdown
Collaborator

@kgioioso kgioioso commented Sep 18, 2023

This PR changes the record logic.

Previously, when a duplicate request is received (either because the client retries the request or the replica receives a missed index), the duplicate entry is added to ProcessQueue. The duplicate entry is then handled depending on the status of the entry.

In this PR, if a duplicate request is received, recordTd checks the status of the entry. If PROCESSED, the entry is routed to fastReplyQu so the reply can be resent. If not processed, the other request is still being processed, and recordTd does nothing with the new request.

This significantly simplifies the logic of processTd because now processTd only handles entries with status INITIAL.

Comment thread lib/common_struct.h

std::string result; // The execution result of the LogEntry
char status; //
char status; // TODO(Katie): does this need to be thread safe?
Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Steamgjk Does this need to be an atomic type? It seems like recordTd could be reading this value while processTd is writing it.

@kgioioso kgioioso requested a review from Steamgjk September 18, 2023 22:25
@kgioioso
Copy link
Copy Markdown
Collaborator Author

@Steamgjk this is the first logic change I have made. I believe this change does not affect correctness, but I may be missing some nuances of the codebase. Can you please double check my logic? Thanks.

@kgioioso kgioioso marked this pull request as ready for review September 19, 2023 00:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant