generated from codecrafters-io/tester-template
-
Notifications
You must be signed in to change notification settings - Fork 2
Refactor sendAndReceive
#64
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Draft
ryan-gang
wants to merge
10
commits into
main
Choose a base branch
from
send-and-receive
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Draft
Changes from 9 commits
Commits
Show all changes
10 commits
Select commit
Hold shift + click to select a range
29ddda6
feat: add hex dump formatter and API key name resolver functions
ryan-gang 850d649
feat: add concurrency-safe Kafka client with connect, send, and recei…
ryan-gang e937707
refactor: unify RequestHeader encoding and simplify request encoding
ryan-gang a9235ba
refactor: update DescribeTopicPartitionsRequest encoding method for c…
ryan-gang e113bfd
refactor: replace Broker with Client for Kafka connection and requests
ryan-gang 0e2f0c8
feat(builder): add RequestI interface with Encode and GetHeader methods
ryan-gang 589813e
refactor: use constant for max retries in ConnectWithRetries function
ryan-gang aea3133
fix(kafka_client): correct retry limit check and use constant for max…
ryan-gang 2bc92ec
refactor: remove deprecated Connect method to simplify client code
ryan-gang 834228f
fix(kafka_client): make createFrom use value receiver for Response
ryan-gang File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,8 @@ | ||
package builder | ||
|
||
import kafkaapi "github.com/codecrafters-io/kafka-tester/protocol/api" | ||
|
||
type RequestI interface { | ||
Encode() []byte | ||
GetHeader() kafkaapi.RequestHeader | ||
} |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,198 @@ | ||
package kafka_client | ||
|
||
import ( | ||
"bytes" | ||
"encoding/binary" | ||
"fmt" | ||
"io" | ||
"net" | ||
"time" | ||
|
||
"github.com/codecrafters-io/kafka-tester/internal/kafka_executable" | ||
"github.com/codecrafters-io/kafka-tester/protocol/builder" | ||
"github.com/codecrafters-io/kafka-tester/protocol/utils" | ||
"github.com/codecrafters-io/tester-utils/logger" | ||
) | ||
|
||
type Response struct { | ||
RawBytes []byte | ||
Payload []byte | ||
} | ||
|
||
func (r *Response) createFrom(lengthResponse []byte, bodyResponse []byte) Response { | ||
return Response{ | ||
RawBytes: append(lengthResponse, bodyResponse...), | ||
Payload: bodyResponse, | ||
} | ||
} | ||
ryan-gang marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
||
// Client represents a single connection to the Kafka broker. | ||
// All operations on this object are entirely concurrency-safe. | ||
type Client struct { | ||
id int32 | ||
addr string | ||
conn net.Conn | ||
} | ||
|
||
// NewClient creates and returns a Client targeting the given host:port address. | ||
// This does not attempt to actually connect, you have to call Open() for that. | ||
func NewClient(addr string) *Client { | ||
return &Client{id: -1, addr: addr} | ||
} | ||
|
||
func (c *Client) ConnectWithRetries(executable *kafka_executable.KafkaExecutable, logger *logger.Logger) error { | ||
const maxRetries = 10 | ||
logger.Debugf("Connecting to broker at: %s", c.addr) | ||
|
||
retries := 0 | ||
var err error | ||
var conn net.Conn | ||
for { | ||
conn, err = net.Dial("tcp", c.addr) | ||
if err != nil && retries >= maxRetries { | ||
logger.Infof("All retries failed. Exiting.") | ||
return err | ||
} | ||
|
||
if err != nil { | ||
if executable.HasExited() { | ||
return fmt.Errorf("Looks like your program has terminated. A Kafka server is expected to be a long-running process.") | ||
} | ||
|
||
// Don't print errors in the first second | ||
// ToDo: fixtures fail | ||
// if retries > 2 { | ||
// logger.Infof("Failed to connect to broker at %s, retrying in 1s", b.addr) | ||
// } | ||
|
||
retries++ | ||
time.Sleep(1000 * time.Millisecond) | ||
} else { | ||
break | ||
} | ||
} | ||
logger.Debugf("Connection to broker at %s successful", c.addr) | ||
c.conn = conn | ||
|
||
return nil | ||
} | ||
|
||
func (c *Client) Close() error { | ||
err := c.conn.Close() | ||
if err != nil { | ||
return fmt.Errorf("Failed to close connection to broker at %s: %s", c.addr, err) | ||
} | ||
return nil | ||
} | ||
|
||
func (c *Client) SendAndReceive(request builder.RequestI, stageLogger *logger.Logger) (Response, error) { | ||
header := request.GetHeader() | ||
apiType := utils.APIKeyToName(header.ApiKey) | ||
apiVersion := header.ApiVersion | ||
correlationId := header.CorrelationId | ||
message := request.Encode() | ||
|
||
stageLogger.Infof("Sending \"%s\" (version: %v) request (Correlation id: %v)", apiType, apiVersion, correlationId) | ||
stageLogger.Debugf("Hexdump of sent \"%s\" request: \n%v\n", apiType, utils.GetFormattedHexdump(message)) | ||
|
||
response := Response{} | ||
|
||
err := c.Send(message) | ||
if err != nil { | ||
return response, err | ||
} | ||
|
||
response, err = c.Receive() | ||
if err != nil { | ||
return response, err | ||
} | ||
|
||
stageLogger.Debugf("Hexdump of received \"%s\" response: \n%v\n", apiType, utils.GetFormattedHexdump(response.RawBytes)) | ||
|
||
return response, nil | ||
} | ||
|
||
func (c *Client) Send(message []byte) error { | ||
// Set a deadline for the write operation | ||
err := c.conn.SetWriteDeadline(time.Now().Add(100 * time.Millisecond)) | ||
if err != nil { | ||
return fmt.Errorf("failed to set write deadline: %v", err) | ||
} | ||
|
||
_, err = c.conn.Write(message) | ||
|
||
// Reset the write deadline | ||
c.conn.SetWriteDeadline(time.Time{}) | ||
|
||
if err != nil { | ||
if netErr, ok := err.(net.Error); ok && netErr.Timeout() { | ||
return fmt.Errorf("write operation timed out") | ||
} | ||
return fmt.Errorf("error writing to connection: %v", err) | ||
} | ||
|
||
return nil | ||
} | ||
|
||
func (c *Client) Receive() (Response, error) { | ||
response := Response{} | ||
|
||
lengthResponse := make([]byte, 4) // length | ||
_, err := io.ReadFull(c.conn, lengthResponse) | ||
if err != nil { | ||
return response, err | ||
} | ||
length := int32(binary.BigEndian.Uint32(lengthResponse)) | ||
|
||
bodyResponse := make([]byte, length) | ||
|
||
// Set a deadline for the read operation | ||
err = c.conn.SetReadDeadline(time.Now().Add(100 * time.Millisecond)) | ||
if err != nil { | ||
return response, fmt.Errorf("failed to set read deadline: %v", err) | ||
} | ||
|
||
numBytesRead, err := io.ReadFull(c.conn, bodyResponse) | ||
|
||
// Reset the read deadline | ||
c.conn.SetReadDeadline(time.Time{}) | ||
|
||
bodyResponse = bodyResponse[:numBytesRead] | ||
|
||
if err != nil { | ||
if netErr, ok := err.(net.Error); ok && netErr.Timeout() { | ||
// If the read timed out, return the partial response we have so far | ||
// This way we can surface a better error message to help w debugging | ||
return response.createFrom(lengthResponse, bodyResponse), nil | ||
} | ||
return response, fmt.Errorf("error reading from connection: %v", err) | ||
} | ||
|
||
return response.createFrom(lengthResponse, bodyResponse), nil | ||
} | ||
ryan-gang marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
||
func (c *Client) ReceiveRaw() ([]byte, error) { | ||
var buf bytes.Buffer | ||
|
||
// Set a deadline for the read operation | ||
err := c.conn.SetReadDeadline(time.Now().Add(100 * time.Millisecond)) | ||
if err != nil { | ||
return nil, fmt.Errorf("failed to set read deadline: %v", err) | ||
} | ||
|
||
// Use a limited reader to prevent reading indefinitely | ||
limitedReader := io.LimitReader(c.conn, 1024*1024) // Limit to 1MB, adjust as needed | ||
_, err = io.Copy(&buf, limitedReader) | ||
|
||
// Reset the read deadline | ||
c.conn.SetReadDeadline(time.Time{}) | ||
|
||
if err != nil && err != io.EOF { | ||
if netErr, ok := err.(net.Error); ok && netErr.Timeout() { | ||
} else { | ||
return nil, fmt.Errorf("error reading from connection: %v", err) | ||
} | ||
} | ||
|
||
return buf.Bytes(), nil | ||
} |
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Will be removed.