Skip to content
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

[3.2.1 Backport] CBG-4201: missing logging for ChangesOptions for Changes requests #7129

Merged
merged 1 commit into from
Sep 26, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 2 additions & 3 deletions db/blip_handler.go
Original file line number Diff line number Diff line change
Expand Up @@ -259,12 +259,11 @@ func (bh *blipHandler) handleSetCheckpoint(rq *blip.Message) error {

// Received a "subChanges" subscription request
func (bh *blipHandler) handleSubChanges(rq *blip.Message) error {
defaultSince := CreateZeroSinceValue()
latestSeq := func() (SequenceID, error) {
seq, err := bh.collection.LastSequence(bh.loggingCtx)
return SequenceID{Seq: seq}, err
}
subChangesParams, err := NewSubChangesParams(bh.loggingCtx, rq, defaultSince, latestSeq, ParseJSONSequenceID)
subChangesParams, err := NewSubChangesParams(bh.loggingCtx, rq, latestSeq, bh.db.Options.ChangesRequestPlus)
if err != nil {
return base.HTTPErrorf(http.StatusBadRequest, "Invalid subChanges parameters")
}
Expand Down Expand Up @@ -318,7 +317,7 @@ func (bh *blipHandler) handleSubChanges(rq *blip.Message) error {
requestPlusSeq := uint64(0)
// If non-continuous, check whether requestPlus handling is set for request or via database config
if continuous == false {
useRequestPlus := subChangesParams.requestPlus(bh.db.Options.ChangesRequestPlus)
useRequestPlus := subChangesParams.requestPlus()
if useRequestPlus {
seq, requestPlusErr := bh.db.GetRequestPlusSequence()
if requestPlusErr != nil {
Expand Down
29 changes: 21 additions & 8 deletions db/blip_sync_messages.go
Original file line number Diff line number Diff line change
Expand Up @@ -147,26 +147,27 @@ type SubChangesParams struct {
_since SequenceID // Since value on the incoming request
_docIDs []string // Document ID filter specified on the incoming request
_sendReplacementRevs bool // Whether to send a replacement rev in the event that we do not find the requested one.
_defaultRequestPlus bool // Default value for requestPlus, set on a database level
}

type SubChangesBody struct {
DocIDs []string `json:"docIDs"`
}

// Create a new subChanges helper
func NewSubChangesParams(logCtx context.Context, rq *blip.Message, zeroSeq SequenceID, latestSeq LatestSequenceFunc, sequenceIDParser SequenceIDParser) (*SubChangesParams, error) {

// NewSubChangesParams creates a SubChangesParam from a request. latestSeq represnts a function to determine the latest sequence for a database. defaultRequestPlus is the default value of request plus for a database.
func NewSubChangesParams(logCtx context.Context, rq *blip.Message, latestSeq LatestSequenceFunc, defaultRequestPlus bool) (*SubChangesParams, error) {
params := &SubChangesParams{
rq: rq,
rq: rq,
_defaultRequestPlus: defaultRequestPlus,
}

// Determine incoming since and docIDs once, since there is some overhead associated with their calculation
sinceSequenceId := zeroSeq
sinceSequenceId := CreateZeroSinceValue()
var err error
if rq.Properties[SubChangesFuture] == trueProperty {
sinceSequenceId, err = latestSeq()
} else if sinceStr, found := rq.Properties[SubChangesSince]; found {
if sinceSequenceId, err = sequenceIDParser(sinceStr); err != nil {
if sinceSequenceId, err = ParseJSONSequenceID(sinceStr); err != nil {
base.InfofCtx(logCtx, base.KeySync, "%s: Invalid sequence ID in 'since': %s", rq, sinceStr)
}
}
Expand Down Expand Up @@ -240,10 +241,10 @@ func (s *SubChangesParams) activeOnly() bool {
return (s.rq.Properties[SubChangesActiveOnly] == trueProperty)
}

func (s *SubChangesParams) requestPlus(defaultValue bool) (value bool) {
func (s *SubChangesParams) requestPlus() (value bool) {
propertyValue, isDefined := s.rq.Properties[SubChangesRequestPlus]
if !isDefined {
return defaultValue
return s._defaultRequestPlus
}
return propertyValue == trueProperty
}
Expand Down Expand Up @@ -300,6 +301,14 @@ func (s *SubChangesParams) String() string {
buffer.WriteString(fmt.Sprintf("BatchSize:%v ", s.batchSize()))
}

requestPlus := s.requestPlus()
if requestPlus {
buffer.WriteString(fmt.Sprintf("RequestPlus:%v ", requestPlus))
}
future, ok := s.rq.Properties[SubChangesFuture]
if ok {
buffer.WriteString(fmt.Sprintf("Future:%v ", future))
}
if len(s.docIDs()) > 0 {
buffer.WriteString(fmt.Sprintf("DocIDs:%v ", s.docIDs()))
}
Expand Down Expand Up @@ -468,6 +477,10 @@ func (rm *RevMessage) String() string {
buffer.WriteString(fmt.Sprintf("Sequence:%v ", sequence))
}

noConflicts, ok := rm.Properties[RevMessageNoConflicts]
if ok {
buffer.WriteString(fmt.Sprintf("NoConflicts:%v ", noConflicts))
}
return buffer.String()

}
Expand Down
3 changes: 2 additions & 1 deletion db/changes.go
Original file line number Diff line number Diff line change
Expand Up @@ -1338,7 +1338,7 @@ func createChangesEntry(ctx context.Context, docid string, db *DatabaseCollectio

func (options ChangesOptions) String() string {
return fmt.Sprintf(
`{Since: %s, Limit: %d, Conflicts: %t, IncludeDocs: %t, Wait: %t, Continuous: %t, HeartbeatMs: %d, TimeoutMs: %d, ActiveOnly: %t, RequestPlusSeq: %d}`,
`{Since: %s, Limit: %d, Conflicts: %t, IncludeDocs: %t, Wait: %t, Continuous: %t, HeartbeatMs: %d, TimeoutMs: %d, ActiveOnly: %t, Revocations: %t, RequestPlusSeq: %d}`,
options.Since,
options.Limit,
options.Conflicts,
Expand All @@ -1348,6 +1348,7 @@ func (options ChangesOptions) String() string {
options.HeartbeatMs,
options.TimeoutMs,
options.ActiveOnly,
options.Revocations,
options.RequestPlusSeq,
)
}
Expand Down
26 changes: 26 additions & 0 deletions db/changes_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,8 @@ import (
"context"
"fmt"
"log"
"reflect"
"strings"
"testing"

"github.com/couchbase/sync_gateway/base"
Expand Down Expand Up @@ -517,3 +519,27 @@ func BenchmarkChangesFeedDocUnmarshalling(b *testing.B) {
}

}

func TestChangesOptionsStringer(t *testing.T) {
opts := ChangesOptions{}
var stringerFields []string
for _, key := range strings.Split(opts.String()[1:len(opts.String())-1], ",") {
fieldName, _, found := strings.Cut(strings.Trim(key, `" ,`), ":")
require.True(t, found, "Expected , in %s", key)
stringerFields = append(stringerFields, fieldName)
}
ignoredFields := map[string]struct{}{
"ChangesCtx": {},
"clientType": {},
}
var expectedFields []string
for _, field := range reflect.VisibleFields(reflect.TypeOf(ChangesOptions{})) {
// some field names are not in stringer
if _, ok := ignoredFields[field.Name]; ok {
continue
}
expectedFields = append(expectedFields, field.Name)
}
require.ElementsMatch(t, expectedFields, stringerFields)

}
4 changes: 2 additions & 2 deletions rest/blip_sync_messages_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -80,7 +80,7 @@ func TestSubChangesSince(t *testing.T) {
rq := blip.NewRequest()
rq.Properties["since"] = `"1"`

subChangesParams, err := db.NewSubChangesParams(base.TestCtx(t), rq, db.SequenceID{}, nil, db.ParseJSONSequenceID)
subChangesParams, err := db.NewSubChangesParams(base.TestCtx(t), rq, nil, rt.GetDatabase().Options.ChangesRequestPlus)
require.NoError(t, err)

seqID := subChangesParams.Since()
Expand All @@ -102,7 +102,7 @@ func TestSubChangesFuture(t *testing.T) {
rq.Properties["future"] = "true"
rq.Properties["since"] = `"1"`

subChangesParams, err := db.NewSubChangesParams(base.TestCtx(t), rq, db.SequenceID{}, latestSeq, db.ParseJSONSequenceID)
subChangesParams, err := db.NewSubChangesParams(base.TestCtx(t), rq, latestSeq, rt.GetDatabase().Options.ChangesRequestPlus)
require.NoError(t, err)

seqID := subChangesParams.Since()
Expand Down
Loading