Skip to content

Commit

Permalink
perf(storagenode): remove backup from append request (#412)
Browse files Browse the repository at this point in the history
### What this PR does

This PR removes the Backups fields from AppendRequest. Thus, it makes unmarshaling AppendRequest can
be improved.
  • Loading branch information
ijsong authored Jun 1, 2023
2 parents da1409d + f75ef55 commit 0abcb1a
Show file tree
Hide file tree
Showing 5 changed files with 61 additions and 140 deletions.
3 changes: 1 addition & 2 deletions internal/storagenode/client/log_client.go
Original file line number Diff line number Diff line change
Expand Up @@ -41,12 +41,11 @@ func (c *LogClient) reset(rpcConn *rpc.Conn, _ types.ClusterID, target varlogpb.
// Append stores data to the log stream specified with the topicID and the logStreamID.
// The backup indicates the storage nodes that have backup replicas of that log stream.
// It returns valid GLSN if the append completes successfully.
func (c *LogClient) Append(ctx context.Context, tpid types.TopicID, lsid types.LogStreamID, data [][]byte, backups ...varlogpb.StorageNode) ([]snpb.AppendResult, error) {
func (c *LogClient) Append(ctx context.Context, tpid types.TopicID, lsid types.LogStreamID, data [][]byte) ([]snpb.AppendResult, error) {
req := &snpb.AppendRequest{
TopicID: tpid,
LogStreamID: lsid,
Payload: data,
Backups: backups,
}
rsp, err := c.rpcClient.Append(ctx, req)
if err != nil {
Expand Down
9 changes: 1 addition & 8 deletions internal/storagenode/testing.go
Original file line number Diff line number Diff line change
Expand Up @@ -132,14 +132,7 @@ func TestAppend(t *testing.T, tpid types.TopicID, lsid types.LogStreamID, dataBa
lc, closer := TestNewLogIOClient(t, replicas[0].StorageNodeID, replicas[0].Address)
defer closer()

var backups []varlogpb.StorageNode
for _, replica := range replicas[1:] {
backups = append(backups, varlogpb.StorageNode{
StorageNodeID: replica.StorageNodeID,
Address: replica.Address,
})
}
res, err := lc.Append(context.Background(), tpid, lsid, dataBatch, backups...)
res, err := lc.Append(context.Background(), tpid, lsid, dataBatch)
assert.NoError(t, err)
return res
}
Expand Down
8 changes: 1 addition & 7 deletions pkg/varlog/operations.go
Original file line number Diff line number Diff line change
Expand Up @@ -102,13 +102,7 @@ func (v *logImpl) appendTo(ctx context.Context, tpid types.TopicID, lsid types.L
return nil, fmt.Errorf("append: %w", err)
}

backup := make([]varlogpb.StorageNode, len(replicas)-1)
for i := 0; i < len(replicas)-1; i++ {
backup[i].StorageNodeID = replicas[i+1].StorageNodeID
backup[i].Address = replicas[i+1].Address
}

res, err := cl.Append(ctx, tpid, lsid, data, backup...)
res, err := cl.Append(ctx, tpid, lsid, data)
if err != nil {
if strings.Contains(err.Error(), "sealed") {
err = fmt.Errorf("append: %s: %w", err.Error(), verrors.ErrSealed)
Expand Down
180 changes: 58 additions & 122 deletions proto/snpb/log_io.pb.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 0 additions & 1 deletion proto/snpb/log_io.proto
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,6 @@ message AppendRequest {
(gogoproto.customname) = "LogStreamID"
];
repeated bytes payload = 3;
repeated varlogpb.StorageNode backups = 4 [(gogoproto.nullable) = false];
}

message AppendResult {
Expand Down

0 comments on commit 0abcb1a

Please sign in to comment.