Skip to content

Conversation

@somdoron
Copy link
Contributor

No description provided.

@somdoron somdoron requested review from Copilot and keisar October 28, 2025 17:29
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR adds Query support to the client-server libraries, enabling read-only operations that bypass the state machine. The implementation includes protocol extensions (Query/QueryResponse messages with correlationId), client retry logic mirroring ClientRequest behavior, and server-side handling. The kvstore example is updated to use Query for GET operations instead of going through the replicated state machine.

Key Changes:

  • Introduces Query/QueryResponse protocol messages with client-generated correlationId for matching responses across retries
  • Implements PendingQueries management with resend logic matching PendingRequests behavior
  • Updates kvstore GET command to use Query-based reads served directly from leader's in-memory state

Reviewed Changes

Copilot reviewed 29 out of 29 changed files in this pull request and generated 3 comments.

Show a summary per file
File Description
specs/005-client-server-libraries/tasks.md Task breakdown for implementing Query support
specs/005-client-server-libraries/spec.md Feature specification defining Query requirements and semantics
specs/005-client-server-libraries/research.md Design decisions for Query implementation
specs/005-client-server-libraries/quickstart.md Usage guide for Query-based GET
specs/005-client-server-libraries/plan.md Implementation plan and phases
specs/005-client-server-libraries/data-model.md Data model for Query entities
kvstore/src/test/scala/zio/kvstore/KVStoreQueryGetSpec.scala Integration test for Query-based GET
kvstore/src/main/scala/zio/kvstore/package.scala Removed Get command (replaced by Query)
kvstore/src/main/scala/zio/kvstore/node/Node.scala Server-side Query handler for GET
kvstore/src/main/scala/zio/kvstore/KVStateMachine.scala Removed Get command processing
kvstore/src/main/scala/zio/kvstore/KVServer.scala Added Query action handling and replyQuery method
kvstore/src/main/scala/zio/kvstore/Codecs.scala Removed Get command codec
kvstore-protocol/src/main/scala/zio/kvstore/protocol/ClientApi.scala Moved Get from ClientRequest to KVQuery
kvstore-cli/src/main/scala/zio/kvstore/cli/KVClient.scala Updated GET to use Query API
client-server-server/src/test/scala/zio/raft/server/QueryServerSpec.scala Server-side Query handling test
client-server-server/src/main/scala/zio/raft/server/RaftServer.scala Server Query handling and QueryResponse delivery
client-server-protocol/src/test/scala/zio/raft/protocol/QueryCodecSpec.scala Protocol codec tests for Query/QueryResponse
client-server-protocol/src/main/scala/zio/raft/protocol/package.scala CorrelationId type definition
client-server-protocol/src/main/scala/zio/raft/protocol/ServerMessages.scala QueryResponse message definition
client-server-protocol/src/main/scala/zio/raft/protocol/Codecs.scala Query/QueryResponse codecs
client-server-protocol/src/main/scala/zio/raft/protocol/ClientMessages.scala Query message definition
client-server-client/src/test/scala/zio/raft/client/RaftClientSpec.scala Client Query submission test
client-server-client/src/test/scala/zio/raft/client/PendingRequestsSpec.scala Fixed Promise type parameters
client-server-client/src/test/scala/zio/raft/client/PendingQueriesSpec.scala Tests for PendingQueries resend logic
client-server-client/src/main/scala/zio/raft/client/package.scala SubmitQuery action definition
client-server-client/src/main/scala/zio/raft/client/RaftClient.scala Query API and state handling
client-server-client/src/main/scala/zio/raft/client/PendingRequests.scala Fixed Promise type parameters
client-server-client/src/main/scala/zio/raft/client/PendingQueries.scala PendingQueries implementation
.cursor/rules/specify-rules.mdc Updated project documentation

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

import java.time.Instant
import zio.kvstore.Codecs.given
import zio.kvstore.given
import zio.kvstore.Codecs.given scodec.Codec[zio.kvstore.KVCommand]
Copy link

Copilot AI Oct 28, 2025

Choose a reason for hiding this comment

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

The import statement on line 16 uses an unusual 'given' import with type specification. This appears redundant since line 15 already imports all givens from the kvstore package. Consider removing line 16 or clarifying why a specific given import is needed.

Suggested change
import zio.kvstore.Codecs.given scodec.Codec[zio.kvstore.KVCommand]

Copilot uses AI. Check for mistakes.
case StreamEvent.Action(ClientAction.SubmitQuery(payload, promise)) =>
for {
now <- Clock.instant
// correlationId via client-side generator (to be implemented in T024)
Copy link

Copilot AI Oct 28, 2025

Choose a reason for hiding this comment

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

The comment references task T024 which is already marked as complete in tasks.md. This comment should be removed or updated since the implementation is now present.

Suggested change
// correlationId via client-side generator (to be implemented in T024)

Copilot uses AI. Check for mistakes.
@somdoron somdoron merged commit a8982ed into unit-finance:main Oct 28, 2025
1 check passed
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