Skip to content

Commit 301297c

Browse files
[3.1.6 backport] CBG-3876 suppress warning for non json objects (#6795)
* [CBG-3876] suppress warning for non json objects backport of CBG-3835 catch error in case there is a valid json but not a JSON object (#6756) * Remove logging for invalid JSON in sync attribute log in the case where there is _sync xattr but the body has become non json * Update log messages * Update db/import_listener.go Co-authored-by: Adam Fraser <adam.fraser@couchbase.com> --------- Co-authored-by: Adam Fraser <adam.fraser@couchbase.com> * Change test to work without multi-xattr write APIs --------- Co-authored-by: Adam Fraser <adam.fraser@couchbase.com>
1 parent f24bda6 commit 301297c

File tree

4 files changed

+138
-12
lines changed

4 files changed

+138
-12
lines changed

db/document.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -412,7 +412,7 @@ func UnmarshalDocumentSyncData(data []byte, needHistory bool) (*SyncData, error)
412412
root.SyncData = &SyncData{History: make(RevTree)}
413413
}
414414
if err := base.JSONUnmarshal(data, &root); err != nil {
415-
return nil, err
415+
return nil, fmt.Errorf("Could not unmarshal _sync out of document body: %w", err)
416416
}
417417
if root.SyncData != nil && root.SyncData.Deleted_OLD {
418418
root.SyncData.Deleted_OLD = false
@@ -447,7 +447,7 @@ func UnmarshalDocumentSyncDataFromFeed(data []byte, dataType uint8, userXattrKey
447447
}
448448
err = base.JSONUnmarshal(syncXattr, result)
449449
if err != nil {
450-
return nil, nil, nil, nil, err
450+
return nil, nil, nil, nil, fmt.Errorf("Found _sync xattr (%q), but could not unmarshal: %w", string(syncXattr), err)
451451
}
452452
return result, body, syncXattr, rawUserXattr, nil
453453
}

db/import.go

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -40,6 +40,7 @@ func (db *DatabaseCollectionWithUser) ImportDocRaw(ctx context.Context, docid st
4040
} else {
4141
err := body.Unmarshal(value)
4242
if err != nil {
43+
db.dbStats().SharedBucketImport().ImportErrorCount.Add(1)
4344
base.InfofCtx(ctx, base.KeyImport, "Unmarshal error during importDoc %v", err)
4445
return nil, base.HTTPErrorf(http.StatusNotFound, "Error unmarshalling %s: %s", base.UD(docid).Redact(), err)
4546
}

db/import_listener.go

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -158,7 +158,7 @@ func (il *importListener) ProcessFeedEvent(event sgbucket.FeedEvent) (shouldPers
158158

159159
// If this is a binary document we can ignore, but update checkpoint to avoid reprocessing upon restart
160160
if event.DataType == base.MemcachedDataTypeRaw {
161-
base.InfofCtx(ctx, base.KeyImport, "Ignoring binary mutation event for %s.", base.UD(docID))
161+
base.DebugfCtx(ctx, base.KeyImport, "Ignoring binary mutation event for %s.", base.UD(docID))
162162
return true
163163
}
164164

@@ -172,10 +172,10 @@ func (il *importListener) ImportFeedEvent(ctx context.Context, collection *Datab
172172
if err != nil {
173173
if err == base.ErrEmptyMetadata {
174174
base.WarnfCtx(ctx, "Unexpected empty metadata when processing feed event. docid: %s opcode: %v datatype:%v", base.UD(event.Key), event.Opcode, event.DataType)
175-
} else {
176-
base.WarnfCtx(ctx, "Found sync metadata, but unable to unmarshal for feed document %q. Will not be imported. Error: %v", base.UD(event.Key), err)
175+
il.importStats.ImportErrorCount.Add(1)
176+
return
177177
}
178-
il.importStats.ImportErrorCount.Add(1)
178+
base.DebugfCtx(ctx, base.KeyImport, "%s will not be imported: %v", base.UD(event.Key), err)
179179
return
180180
}
181181

db/import_test.go

Lines changed: 131 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -550,8 +550,9 @@ func TestImportNonZeroStart(t *testing.T) {
550550
require.Equal(t, revID1, doc.SyncData.CurrentRev)
551551
}
552552

553-
// TestImportInvalidMetadata tests triggering an import error if the metadata is unmarshalable
554-
func TestImportInvalidMetadata(t *testing.T) {
553+
// TestImportFeedInvalidInlineSyncMetadata tests avoiding an import error if the metadata is unmarshable
554+
func TestImportFeedInvalidInlineSyncMetadata(t *testing.T) {
555+
base.SetUpTestLogging(t, base.LevelDebug, base.KeyMigrate, base.KeyImport)
555556
base.SkipImportTestsIfNotEnabled(t)
556557
bucket := base.GetTestBucket(t)
557558
defer bucket.Close(base.TestCtx(t))
@@ -563,13 +564,137 @@ func TestImportInvalidMetadata(t *testing.T) {
563564
require.Equal(t, int64(0), db.DbStats.SharedBucketImport().ImportCount.Value())
564565
require.Equal(t, int64(0), db.DbStats.SharedBucketImport().ImportErrorCount.Value())
565566

566-
// write a document with inline sync metadata that is unmarshalable, triggering an import error
567-
// can't write a document with invalid sync metadata as an xattr, so rely on legacy behavior
568-
_, err := bucket.GetSingleDataStore().Add("doc1", 0, `{"foo" : "bar", "_sync" : 1 }`)
567+
// docs named so they will both be on vBucket 1 in both 64 and 1024 vbuckets
568+
const (
569+
doc1 = "bookstand"
570+
doc2 = "chipchop"
571+
)
572+
// write a document with inline sync metadata that not unmarshalable into SyncData. This document will be ignored and logged at debug level.
573+
// [DBG] .. col:sg_test_0 <ud>bookstand</ud> not able to be imported. Error: Could not unmarshal _sync out of document body: json: cannot unmarshal number into Go struct field documentRoot._sync of type db.SyncData
574+
_, err := bucket.GetSingleDataStore().Add(doc1, 0, []byte(`{"foo" : "bar", "_sync" : 1 }`))
575+
require.NoError(t, err)
576+
577+
// this will be imported
578+
err = bucket.GetSingleDataStore().Set(doc2, 0, nil, []byte(`{"foo" : "bar"}`))
579+
require.NoError(t, err)
580+
581+
base.RequireWaitForStat(t, func() int64 {
582+
return db.DbStats.SharedBucketImport().ImportCount.Value()
583+
}, 1)
584+
require.Equal(t, int64(0), db.DbStats.SharedBucketImport().ImportErrorCount.Value())
585+
}
586+
587+
func TestImportFeedInvalidSyncMetadata(t *testing.T) {
588+
base.SetUpTestLogging(t, base.LevelDebug, base.KeyMigrate, base.KeyImport)
589+
base.SkipImportTestsIfNotEnabled(t)
590+
bucket := base.GetTestBucket(t)
591+
ctx := base.TestCtx(t)
592+
defer bucket.Close(ctx)
593+
594+
col, ok := bucket.GetSingleDataStore().(*base.Collection)
595+
require.True(t, ok)
596+
597+
// docs named so they will both be on vBucket 1 in both 64 and 1024 vbuckets
598+
const (
599+
doc1 = "bookstand"
600+
doc2 = "chipchop"
601+
602+
exp = 0
603+
)
604+
605+
// perform doc writes in two parts (Add + WriteUserXattr) since WriteWithXattr does cas expansion on _sync, to write the document and the unexpanded xattr
606+
607+
// this document will be ignored for input with debug logging as follows:
608+
// [DBG] .. col:sg_test_0 <ud>bookstand</ud> not able to be imported. Error: Found _sync xattr ("1"), but could not unmarshal: json: cannot unmarshal number into Go value of type db.SyncData
609+
_, err := col.Add(doc1, exp, []byte(`{"foo" : "bar"}`))
610+
require.NoError(t, err)
611+
_, err = col.WriteUserXattr(doc1, base.SyncXattrName, 1)
612+
require.NoError(t, err)
613+
614+
// fix xattrs, and the document is able to be imported
615+
_, err = col.Add(doc2, exp, []byte(`{"foo": "bar"}`))
616+
require.NoError(t, err)
617+
_, err = col.WriteUserXattr(doc2, base.SyncXattrName, []byte(`{}`))
618+
require.NoError(t, err)
619+
620+
db, ctx := setupTestDBWithOptionsAndImport(t, bucket, DatabaseContextOptions{})
621+
defer db.Close(ctx)
622+
623+
base.RequireWaitForStat(t, func() int64 {
624+
return db.DbStats.SharedBucketImport().ImportCount.Value()
625+
}, 1)
626+
require.Equal(t, int64(0), db.DbStats.SharedBucketImport().ImportErrorCount.Value())
627+
}
628+
629+
func TestImportFeedNonJSONNewDoc(t *testing.T) {
630+
base.SetUpTestLogging(t, base.LevelDebug, base.KeyMigrate, base.KeyImport)
631+
base.SkipImportTestsIfNotEnabled(t)
632+
bucket := base.GetTestBucket(t)
633+
defer bucket.Close(base.TestCtx(t))
634+
635+
db, ctx := setupTestDBWithOptionsAndImport(t, bucket, DatabaseContextOptions{})
636+
defer db.Close(ctx)
637+
638+
// make sure no documents are imported
639+
require.Equal(t, int64(0), db.DbStats.SharedBucketImport().ImportCount.Value())
640+
require.Equal(t, int64(0), db.DbStats.SharedBucketImport().ImportErrorCount.Value())
641+
642+
// docs named so they will both be on vBucket 1 in both 64 and 1024 vbuckets
643+
const (
644+
doc1 = "bookstand"
645+
doc2 = "chipchop"
646+
)
647+
648+
// logs because a JSON number is not a JSON object
649+
// [DBG] .. col:sg_test_0 <ud>bookstand</ud> not able to be imported. Error: Could not unmarshal _sync out of document body: json: cannot unmarshal number into Go value of type db.documentRoot
650+
_, err := bucket.GetSingleDataStore().Add(doc1, 0, []byte(`1`))
651+
require.NoError(t, err)
652+
653+
_, err = bucket.GetSingleDataStore().Add(doc2, 0, []byte(`{"foo" : "bar"}`))
569654
require.NoError(t, err)
570655

571656
base.RequireWaitForStat(t, func() int64 {
572-
return db.DbStats.SharedBucketImport().ImportErrorCount.Value()
657+
return db.DbStats.SharedBucketImport().ImportCount.Value()
573658
}, 1)
659+
require.Equal(t, int64(0), db.DbStats.SharedBucketImport().ImportErrorCount.Value())
660+
}
661+
662+
func TestImportFeedNonJSONExistingDoc(t *testing.T) {
663+
base.SetUpTestLogging(t, base.LevelDebug, base.KeyCRUD, base.KeyMigrate, base.KeyImport)
664+
base.SkipImportTestsIfNotEnabled(t)
665+
bucket := base.GetTestBucket(t)
666+
defer bucket.Close(base.TestCtx(t))
667+
668+
db, ctx := setupTestDBWithOptionsAndImport(t, bucket, DatabaseContextOptions{})
669+
defer db.Close(ctx)
670+
671+
// make sure no documents are imported
574672
require.Equal(t, int64(0), db.DbStats.SharedBucketImport().ImportCount.Value())
673+
require.Equal(t, int64(0), db.DbStats.SharedBucketImport().ImportErrorCount.Value())
674+
675+
// docs named so they will both be on vBucket 1 in both 64 and 1024 vbuckets
676+
const (
677+
doc1 = "bookstand"
678+
doc2 = "chipchop"
679+
)
680+
681+
_, err := bucket.GetSingleDataStore().Add(doc1, 0, []byte(`{"foo": "bar"}`))
682+
require.NoError(t, err)
683+
684+
base.RequireWaitForStat(t, func() int64 {
685+
return db.DbStats.SharedBucketImport().ImportCount.Value()
686+
}, 1)
687+
688+
// logs and increments ImportErrorCount
689+
// [INF] .. col:sg_test_0 Unmarshal error during importDoc json: cannot unmarshal number into Go value of type db.Body
690+
err = bucket.GetSingleDataStore().Set(doc1, 0, nil, []byte(`1`))
691+
require.NoError(t, err)
692+
693+
_, err = bucket.GetSingleDataStore().Add(doc2, 0, []byte(`{"foo" : "bar"}`))
694+
require.NoError(t, err)
695+
696+
base.RequireWaitForStat(t, func() int64 {
697+
return db.DbStats.SharedBucketImport().ImportCount.Value()
698+
}, 2)
699+
require.Equal(t, int64(1), db.DbStats.SharedBucketImport().ImportErrorCount.Value())
575700
}

0 commit comments

Comments
 (0)