Skip to content

Commit

Permalink
kv: remove withMarshalingDebugging function
Browse files Browse the repository at this point in the history
This debugging function was removed when we fixed cockroachdb#34241 and added back
in when cockroachdb#35803 appeared because it was clear that we hadn't fully fixed
the issue. It's been about 2 months since cockroachdb#35762 merged and we haven't
seen any issues since, so this can now be removed.

I don't think we meant to keep this in for the 19.1 release. We should
backport this commit.

Release note: None
  • Loading branch information
nvanbenschoten committed May 17, 2019
1 parent e6950d7 commit 1c0a1ca
Show file tree
Hide file tree
Showing 2 changed files with 1 addition and 65 deletions.
38 changes: 1 addition & 37 deletions pkg/kv/transport.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,18 +17,14 @@ package kv

import (
"context"
"encoding/hex"
"fmt"
"sort"
"strings"
"time"

"github.com/cockroachdb/cockroach/pkg/internal/client"
"github.com/cockroachdb/cockroach/pkg/roachpb"
"github.com/cockroachdb/cockroach/pkg/rpc"
"github.com/cockroachdb/cockroach/pkg/rpc/nodedialer"
"github.com/cockroachdb/cockroach/pkg/util/log"
"github.com/cockroachdb/cockroach/pkg/util/protoutil"
"github.com/cockroachdb/cockroach/pkg/util/timeutil"
"github.com/cockroachdb/cockroach/pkg/util/tracing"
"github.com/opentracing/opentracing-go"
Expand Down Expand Up @@ -156,34 +152,6 @@ func (gt *grpcTransport) maybeResurrectRetryablesLocked() bool {
return len(resurrect) > 0
}

func withMarshalingDebugging(ctx context.Context, ba roachpb.BatchRequest, f func()) {
nPre := ba.Size()
defer func() {
nPost := ba.Size()
if r := recover(); r != nil || nPre != nPost {
var buf strings.Builder
_, _ = fmt.Fprintf(&buf, "batch size %d -> %d bytes\n", nPre, nPost)
func() {
defer func() {
if rInner := recover(); rInner != nil {
_, _ = fmt.Fprintln(&buf, "panic while re-marshaling:", rInner)
}
}()
data, mErr := protoutil.Marshal(&ba)
if mErr != nil {
_, _ = fmt.Fprintln(&buf, "while re-marshaling:", mErr)
} else {
_, _ = fmt.Fprintln(&buf, "re-marshaled protobuf:")
_, _ = fmt.Fprintln(&buf, hex.Dump(data))
}
}()
_, _ = fmt.Fprintln(&buf, "original panic: ", r)
panic(buf.String())
}
}()
f()
}

// SendNext invokes the specified RPC on the supplied client when the
// client is ready. On success, the reply is sent on the channel;
// otherwise an error is sent.
Expand All @@ -197,11 +165,7 @@ func (gt *grpcTransport) SendNext(
}

ba.Replica = client.replica
var reply *roachpb.BatchResponse

withMarshalingDebugging(ctx, ba, func() {
reply, err = gt.sendBatch(ctx, client.replica.NodeID, iface, ba)
})
reply, err := gt.sendBatch(ctx, client.replica.NodeID, iface, ba)

// NotLeaseHolderErrors can be retried.
var retryable bool
Expand Down
28 changes: 0 additions & 28 deletions pkg/kv/transport_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,6 @@ import (
"github.com/cockroachdb/cockroach/pkg/util/log"
"github.com/cockroachdb/cockroach/pkg/util/tracing"
opentracing "github.com/opentracing/opentracing-go"
"github.com/stretchr/testify/assert"
"google.golang.org/grpc"
)

Expand Down Expand Up @@ -173,30 +172,3 @@ func (m *mockInternalClient) RangeFeed(
) (roachpb.Internal_RangeFeedClient, error) {
return nil, fmt.Errorf("unsupported RangeFeed call")
}

func TestWithMarshalingDebugging(t *testing.T) {
defer leaktest.AfterTest(t)()
t.Skip(fmt.Sprintf("Skipped until #34241 is resolved"))

ctx := context.Background()

exp := `batch size 23 -> 28 bytes
re-marshaled protobuf:
00000000 0a 0a 0a 00 12 06 08 00 10 00 18 00 12 0e 3a 0c |..............:.|
00000010 0a 0a 1a 03 66 6f 6f 22 03 62 61 72 |....foo".bar|
original panic: <nil>
`

assert.PanicsWithValue(t, exp, func() {
var ba roachpb.BatchRequest
ba.Add(&roachpb.ScanRequest{
RequestHeader: roachpb.RequestHeader{
Key: []byte("foo"),
},
})
withMarshalingDebugging(ctx, ba, func() {
ba.Requests[0].GetInner().(*roachpb.ScanRequest).EndKey = roachpb.Key("bar")
})
})
}

0 comments on commit 1c0a1ca

Please sign in to comment.