From 6e57f34ab43cb978e53b00ebab488b113796860d Mon Sep 17 00:00:00 2001 From: Peter Broadhurst Date: Wed, 9 Mar 2022 17:14:56 -0500 Subject: [PATCH 01/11] Add batch writing of messages and data Signed-off-by: Peter Broadhurst --- internal/config/config.go | 13 +- internal/data/data_manager.go | 86 +++--- internal/data/data_manager_test.go | 272 +++++++++++++----- internal/data/message_writer.go | 207 +++++++++++++ internal/data/message_writer_test.go | 135 +++++++++ internal/database/sqlcommon/data_sql.go | 109 ++++--- internal/database/sqlcommon/data_sql_test.go | 55 ++++ internal/database/sqlcommon/message_sql.go | 81 ++++-- .../database/sqlcommon/message_sql_test.go | 55 ++++ internal/database/sqlcommon/provider.go | 2 + .../database/sqlcommon/provider_mock_test.go | 6 +- internal/database/sqlcommon/sqlcommon.go | 37 ++- internal/database/sqlcommon/sqlcommon_test.go | 23 ++ internal/i18n/en_translations.go | 2 + internal/orchestrator/orchestrator.go | 4 + mocks/databasemocks/plugin.go | 28 ++ mocks/datamocks/manager.go | 5 + pkg/database/plugin.go | 8 +- 18 files changed, 953 insertions(+), 175 deletions(-) create mode 100644 internal/data/message_writer.go create mode 100644 internal/data/message_writer_test.go diff --git a/internal/config/config.go b/internal/config/config.go index 730ea1c3c4..cda9d78338 100644 --- a/internal/config/config.go +++ b/internal/config/config.go @@ -188,6 +188,12 @@ var ( MessageCacheSize = rootKey("message.cache.size") // MessageCacheTTL MessageCacheTTL = rootKey("message.cache.ttl") + // MessageWriterCount + MessageWriterCount = rootKey("message.writer.count") + // MessageWriterBatchTimeout + MessageWriterBatchTimeout = rootKey("message.writer.batchTimeout") + // MessageWriterBatchMaxInserts + MessageWriterBatchMaxInserts = rootKey("message.writer.batchMaxInserts") // MetricsEnabled determines whether metrics will be instrumented and if the metrics server will be enabled or not MetricsEnabled = rootKey("metrics.enabled") // MetricsPath determines what path to serve the Prometheus metrics from @@ -338,6 +344,11 @@ func Reset() { viper.SetDefault(string(LogFilesize), "100m") viper.SetDefault(string(LogMaxAge), "24h") viper.SetDefault(string(LogMaxBackups), 2) + viper.SetDefault(string(MessageCacheSize), "50Mb") + viper.SetDefault(string(MessageCacheTTL), "5m") + viper.SetDefault(string(MessageWriterBatchMaxInserts), 200) + viper.SetDefault(string(MessageWriterBatchTimeout), "100ms") + viper.SetDefault(string(MessageWriterCount), 1) viper.SetDefault(string(NamespacesDefault), "default") viper.SetDefault(string(NamespacesPredefined), fftypes.JSONObjectArray{{"name": "default", "description": "Default predefined namespace"}}) viper.SetDefault(string(OrchestratorStartupAttempts), 5) @@ -357,8 +368,6 @@ func Reset() { viper.SetDefault(string(UIEnabled), true) viper.SetDefault(string(ValidatorCacheSize), "1Mb") viper.SetDefault(string(ValidatorCacheTTL), "1h") - viper.SetDefault(string(MessageCacheSize), "50Mb") - viper.SetDefault(string(MessageCacheTTL), "5m") viper.SetDefault(string(IdentityManagerCacheLimit), 100 /* items */) viper.SetDefault(string(IdentityManagerCacheTTL), "1h") diff --git a/internal/data/data_manager.go b/internal/data/data_manager.go index a621f21881..77fdfc7cb4 100644 --- a/internal/data/data_manager.go +++ b/internal/data/data_manager.go @@ -39,8 +39,9 @@ type Manager interface { GetMessageDataCached(ctx context.Context, msg *fftypes.Message, options ...CacheReadOption) (data fftypes.DataArray, foundAll bool, err error) UpdateMessageCache(msg *fftypes.Message, data fftypes.DataArray) UpdateMessageIfCached(ctx context.Context, msg *fftypes.Message) - ResolveInlineDataPrivate(ctx context.Context, ns string, inData fftypes.InlineData) (fftypes.DataArray, error) - ResolveInlineDataBroadcast(ctx context.Context, ns string, inData fftypes.InlineData) (fftypes.DataArray, []*fftypes.DataAndBlob, error) + ResolveInlineDataPrivate(ctx context.Context, msg *NewMessage) error + ResolveInlineDataBroadcast(ctx context.Context, msg *NewMessage) error + WriteNewMessage(ctx context.Context, newMsg *NewMessage) error VerifyNamespaceExists(ctx context.Context, ns string) error UploadJSON(ctx context.Context, ns string, inData *fftypes.DataRefOrValue) (*fftypes.Data, error) @@ -48,6 +49,7 @@ type Manager interface { CopyBlobPStoDX(ctx context.Context, data *fftypes.Data) (blob *fftypes.Blob, err error) DownloadBLOB(ctx context.Context, ns, dataID string) (*fftypes.Blob, io.ReadCloser, error) HydrateBatch(ctx context.Context, persistedBatch *fftypes.BatchPersisted) (*fftypes.Batch, error) + Close() } type dataManager struct { @@ -58,6 +60,7 @@ type dataManager struct { validatorCacheTTL time.Duration messageCache *ccache.Cache messageCacheTTL time.Duration + messageWriter *messageWriter } type messageCacheEntry struct { @@ -114,6 +117,12 @@ func NewDataManager(ctx context.Context, di database.Plugin, pi sharedstorage.Pl ccache.Configure(). MaxSize(config.GetByteSize(config.MessageCacheSize)), ) + dm.messageWriter = newMessageWriter(ctx, di, &messageWriterConf{ + workerCount: config.GetInt(config.MessageWriterCount), + batchTimeout: config.GetDuration(config.MessageWriterBatchTimeout), + maxInserts: config.GetInt(config.MessageWriterBatchMaxInserts), + }) + dm.messageWriter.start() return dm, nil } @@ -365,7 +374,12 @@ func (dm *dataManager) checkValidation(ctx context.Context, ns string, validator return nil } -func (dm *dataManager) validateAndStore(ctx context.Context, ns string, validator fftypes.ValidatorType, datatype *fftypes.DatatypeRef, value *fftypes.JSONAny, blobRef *fftypes.BlobRef) (data *fftypes.Data, blob *fftypes.Blob, err error) { +func (dm *dataManager) validateInputData(ctx context.Context, ns string, inData *fftypes.DataRefOrValue) (data *fftypes.Data, blob *fftypes.Blob, err error) { + + validator := inData.Validator + datatype := inData.Datatype + value := inData.Value + blobRef := inData.Blob if err := dm.checkValidation(ctx, ns, validator, datatype, value); err != nil { return nil, nil, err @@ -384,48 +398,43 @@ func (dm *dataManager) validateAndStore(ctx context.Context, ns string, validato Blob: blobRef, } err = data.Seal(ctx, blob) - if err == nil { - err = dm.database.UpsertData(ctx, data, database.UpsertOptimizationNew) - } if err != nil { return nil, nil, err } return data, blob, nil } -func (dm *dataManager) validateAndStoreInlined(ctx context.Context, ns string, value *fftypes.DataRefOrValue) (*fftypes.Data, *fftypes.Blob, error) { - data, blob, err := dm.validateAndStore(ctx, ns, value.Validator, value.Datatype, value.Value, value.Blob) +func (dm *dataManager) UploadJSON(ctx context.Context, ns string, inData *fftypes.DataRefOrValue) (*fftypes.Data, error) { + data, _, err := dm.validateInputData(ctx, ns, inData) if err != nil { - return nil, nil, err + return nil, err + } + if err = dm.messageWriter.WriteData(ctx, data); err != nil { + return nil, err } - - // Return a ref to the newly saved data - return data, blob, nil -} - -func (dm *dataManager) UploadJSON(ctx context.Context, ns string, inData *fftypes.DataRefOrValue) (*fftypes.Data, error) { - data, _, err := dm.validateAndStore(ctx, ns, inData.Validator, inData.Datatype, inData.Value, inData.Blob) return data, err } -func (dm *dataManager) ResolveInlineDataPrivate(ctx context.Context, ns string, inData fftypes.InlineData) (fftypes.DataArray, error) { - data, _, err := dm.resolveInlineData(ctx, ns, inData, false) - return data, err +func (dm *dataManager) ResolveInlineDataPrivate(ctx context.Context, newMessage *NewMessage) error { + return dm.resolveInlineData(ctx, newMessage, false) } // ResolveInlineDataBroadcast ensures the data object are stored, and returns a list of any data that does not currently // have a shared storage reference, and hence must be published to sharedstorage before a broadcast message can be sent. // We deliberately do NOT perform those publishes inside of this action, as we expect to be in a RunAsGroup (trnasaction) // at this point, and hence expensive things like a multi-megabyte upload should be decoupled by our caller. -func (dm *dataManager) ResolveInlineDataBroadcast(ctx context.Context, ns string, inData fftypes.InlineData) (data fftypes.DataArray, dataToPublish []*fftypes.DataAndBlob, err error) { - return dm.resolveInlineData(ctx, ns, inData, true) +func (dm *dataManager) ResolveInlineDataBroadcast(ctx context.Context, newMessage *NewMessage) error { + return dm.resolveInlineData(ctx, newMessage, true) } -func (dm *dataManager) resolveInlineData(ctx context.Context, ns string, inData fftypes.InlineData, broadcast bool) (data fftypes.DataArray, dataToPublish []*fftypes.DataAndBlob, err error) { +func (dm *dataManager) resolveInlineData(ctx context.Context, newMessage *NewMessage, broadcast bool) (err error) { - data = make(fftypes.DataArray, len(inData)) + r := &newMessage.ResolvedData + inData := newMessage.InData + msg := newMessage.Message + r.AllData = make(fftypes.DataArray, len(newMessage.InData)) if broadcast { - dataToPublish = make([]*fftypes.DataAndBlob, 0, len(inData)) + r.DataToPublish = make([]*fftypes.DataAndBlob, 0, len(inData)) } for i, dataOrValue := range inData { var d *fftypes.Data @@ -433,37 +442,38 @@ func (dm *dataManager) resolveInlineData(ctx context.Context, ns string, inData switch { case dataOrValue.ID != nil: // If an ID is supplied, then it must be a reference to existing data - d, err = dm.resolveRef(ctx, ns, &dataOrValue.DataRef) + d, err = dm.resolveRef(ctx, msg.Header.Namespace, &dataOrValue.DataRef) if err != nil { - return nil, nil, err + return err } if d == nil { - return nil, nil, i18n.NewError(ctx, i18n.MsgDataReferenceUnresolvable, i) + return i18n.NewError(ctx, i18n.MsgDataReferenceUnresolvable, i) } if blob, err = dm.resolveBlob(ctx, d.Blob); err != nil { - return nil, nil, err + return err } case dataOrValue.Value != nil || dataOrValue.Blob != nil: // We've got a Value, so we can validate + store it - if d, blob, err = dm.validateAndStoreInlined(ctx, ns, dataOrValue); err != nil { - return nil, nil, err + if d, blob, err = dm.validateInputData(ctx, msg.Header.Namespace, dataOrValue); err != nil { + return err } + r.NewData = append(r.NewData, d) default: // We have nothing - this must be a mistake - return nil, nil, i18n.NewError(ctx, i18n.MsgDataMissing, i) + return i18n.NewError(ctx, i18n.MsgDataMissing, i) } - data[i] = d + r.AllData[i] = d // If the data is being resolved for public broadcast, and there is a blob attachment, that blob // needs to be published by our calller if broadcast && blob != nil && d.Blob.Public == "" { - dataToPublish = append(dataToPublish, &fftypes.DataAndBlob{ + r.DataToPublish = append(r.DataToPublish, &fftypes.DataAndBlob{ Data: d, Blob: blob, }) } } - return data, dataToPublish, nil + return nil } // HydrateBatch fetches the full messages for a persisted batch, ready for transmission @@ -504,3 +514,11 @@ func (dm *dataManager) HydrateBatch(ctx context.Context, persistedBatch *fftypes return batch, nil } + +func (dm *dataManager) WriteNewMessage(ctx context.Context, newMsg *NewMessage) error { + return dm.messageWriter.WriteNewMessage(ctx, newMsg) +} + +func (dm *dataManager) Close() { + dm.messageWriter.close() +} diff --git a/internal/data/data_manager_test.go b/internal/data/data_manager_test.go index 75de334014..5a89a7207c 100644 --- a/internal/data/data_manager_test.go +++ b/internal/data/data_manager_test.go @@ -32,13 +32,36 @@ import ( ) func newTestDataManager(t *testing.T) (*dataManager, context.Context, func()) { + config.Reset() ctx, cancel := context.WithCancel(context.Background()) mdi := &databasemocks.Plugin{} + mdi.On("Capabilities").Return(&database.Capabilities{ + Concurrency: true, + }) mdx := &dataexchangemocks.Plugin{} mps := &sharedstoragemocks.Plugin{} dm, err := NewDataManager(ctx, mdi, mps, mdx) assert.NoError(t, err) - return dm.(*dataManager), ctx, cancel + return dm.(*dataManager), ctx, func() { + cancel() + dm.Close() + } +} + +func testNewMessage() (*fftypes.UUID, *fftypes.Bytes32, *NewMessage) { + dataID := fftypes.NewUUID() + dataHash := fftypes.NewRandB32() + return dataID, dataHash, &NewMessage{ + Message: &fftypes.Message{ + Header: fftypes.MessageHeader{ + ID: fftypes.NewUUID(), + Namespace: "ns1", + }, + }, + InData: fftypes.InlineData{ + {DataRef: fftypes.DataRef{ID: dataID, Hash: dataHash}}, + }, + } } func TestValidateE2E(t *testing.T) { @@ -92,6 +115,98 @@ func TestValidateE2E(t *testing.T) { } +func TestWriteNewMessageE2E(t *testing.T) { + + config.Reset() + dm, ctx, cancel := newTestDataManager(t) + defer cancel() + + dt := &fftypes.Datatype{ + ID: fftypes.NewUUID(), + Validator: fftypes.ValidatorTypeJSON, + Value: fftypes.JSONAnyPtr(`{}`), + Name: "customer", + Namespace: "0.0.1", + } + + mdi := dm.database.(*databasemocks.Plugin) + mdi.On("GetDatatypeByName", mock.Anything, "ns1", "customer", "0.0.1").Return(dt, nil).Once() + mdi.On("RunAsGroup", mock.Anything, mock.Anything).Run(func(args mock.Arguments) { + err := args[1].(func(context.Context) error)(ctx) + assert.NoError(t, err) + }).Return(nil) + mdi.On("InsertDataArray", mock.Anything, mock.Anything).Return(nil).Once() + + data1, err := dm.UploadJSON(ctx, "ns1", &fftypes.DataRefOrValue{ + Value: fftypes.JSONAnyPtr(`"message 1 - data A"`), + Validator: fftypes.ValidatorTypeJSON, + Datatype: &fftypes.DatatypeRef{ + Name: "customer", + Version: "0.0.1", + }, + }) + assert.NoError(t, err) + + mdi.On("GetDataByID", mock.Anything, data1.ID, true).Return(data1, nil).Once() + + _, _, newMsg1 := testNewMessage() + newMsg1.InData = fftypes.InlineData{ + {DataRef: fftypes.DataRef{ + ID: data1.ID, + Hash: data1.Hash, + }}, + {Value: fftypes.JSONAnyPtr(`"message 1 - data B"`)}, + {Value: fftypes.JSONAnyPtr(`"message 1 - data C"`)}, + } + _, _, newMsg2 := testNewMessage() + newMsg2.InData = fftypes.InlineData{ + {Value: fftypes.JSONAnyPtr(`"message 2 - data B"`)}, + {Value: fftypes.JSONAnyPtr(`"message 2 - data C"`)}, + } + + err = dm.ResolveInlineDataPrivate(ctx, newMsg1) + assert.NoError(t, err) + err = dm.ResolveInlineDataPrivate(ctx, newMsg2) + assert.NoError(t, err) + + allData := append(append(fftypes.DataArray{}, newMsg1.ResolvedData.NewData...), newMsg2.ResolvedData.NewData...) + assert.Len(t, allData, 4) + + mdi.On("InsertMessages", mock.Anything, mock.MatchedBy(func(msgs []*fftypes.Message) bool { + msgsByID := make(map[fftypes.UUID]bool) + for _, msg := range msgs { + msgsByID[*msg.Header.ID] = true + } + return len(msgs) == 2 && + msgsByID[*newMsg1.Message.Header.ID] && + msgsByID[*newMsg2.Message.Header.ID] + })).Return(nil).Once() + mdi.On("InsertDataArray", mock.Anything, mock.MatchedBy(func(dataArray fftypes.DataArray) bool { + dataByID := make(map[fftypes.UUID]bool) + for _, data := range dataArray { + dataByID[*data.ID] = true + } + return len(dataArray) == 4 && + dataByID[*newMsg1.ResolvedData.AllData[1].ID] && + dataByID[*newMsg1.ResolvedData.AllData[2].ID] && + dataByID[*newMsg2.ResolvedData.AllData[0].ID] && + dataByID[*newMsg2.ResolvedData.AllData[1].ID] + })).Return(nil).Once() + + results := make(chan error) + + go func() { + results <- dm.WriteNewMessage(ctx, newMsg1) + }() + go func() { + results <- dm.WriteNewMessage(ctx, newMsg2) + }() + + assert.NoError(t, <-results) + assert.NoError(t, <-results) + + mdi.AssertExpectations(t) +} func TestInitBadDeps(t *testing.T) { _, err := NewDataManager(context.Background(), nil, nil, nil) assert.Regexp(t, "FF10128", err) @@ -266,9 +381,20 @@ func TestResolveInlineDataEmpty(t *testing.T) { dm, ctx, cancel := newTestDataManager(t) defer cancel() - data, err := dm.ResolveInlineDataPrivate(ctx, "ns1", fftypes.InlineData{}) + + newMsg := &NewMessage{ + Message: &fftypes.Message{ + Header: fftypes.MessageHeader{ + ID: fftypes.NewUUID(), + Namespace: "ns1", + }, + }, + InData: fftypes.InlineData{}, + } + + err := dm.ResolveInlineDataPrivate(ctx, newMsg) assert.NoError(t, err) - assert.Empty(t, data) + assert.Empty(t, newMsg.ResolvedData.AllData) } @@ -277,8 +403,7 @@ func TestResolveInlineDataRefIDOnlyOK(t *testing.T) { defer cancel() mdi := dm.database.(*databasemocks.Plugin) - dataID := fftypes.NewUUID() - dataHash := fftypes.NewRandB32() + dataID, dataHash, newMsg := testNewMessage() mdi.On("GetDataByID", ctx, dataID, true).Return(&fftypes.Data{ ID: dataID, @@ -286,13 +411,13 @@ func TestResolveInlineDataRefIDOnlyOK(t *testing.T) { Hash: dataHash, }, nil) - data, err := dm.ResolveInlineDataPrivate(ctx, "ns1", fftypes.InlineData{ - {DataRef: fftypes.DataRef{ID: dataID}}, - }) + err := dm.ResolveInlineDataPrivate(ctx, newMsg) assert.NoError(t, err) - assert.Len(t, data, 1) - assert.Equal(t, dataID, data[0].ID) - assert.Equal(t, dataHash, data[0].Hash) + assert.Len(t, newMsg.ResolvedData.AllData, 1) + assert.Equal(t, dataID, newMsg.ResolvedData.AllData[0].ID) + assert.Equal(t, dataHash, newMsg.ResolvedData.AllData[0].Hash) + assert.Empty(t, newMsg.ResolvedData.NewData) + assert.Empty(t, newMsg.ResolvedData.DataToPublish) } func TestResolveInlineDataBroadcastDataToPublish(t *testing.T) { @@ -300,8 +425,7 @@ func TestResolveInlineDataBroadcastDataToPublish(t *testing.T) { defer cancel() mdi := dm.database.(*databasemocks.Plugin) - dataID := fftypes.NewUUID() - dataHash := fftypes.NewRandB32() + dataID, dataHash, newMsg := testNewMessage() blobHash := fftypes.NewRandB32() mdi.On("GetDataByID", ctx, dataID, true).Return(&fftypes.Data{ @@ -317,16 +441,16 @@ func TestResolveInlineDataBroadcastDataToPublish(t *testing.T) { PayloadRef: "blob/1", }, nil) - refs, dtp, err := dm.ResolveInlineDataBroadcast(ctx, "ns1", fftypes.InlineData{ - {DataRef: fftypes.DataRef{ID: dataID}}, - }) + err := dm.ResolveInlineDataBroadcast(ctx, newMsg) assert.NoError(t, err) - assert.Len(t, refs, 1) - assert.Equal(t, dataID, refs[0].ID) - assert.Equal(t, dataHash, refs[0].Hash) - assert.Len(t, dtp, 1) - assert.Equal(t, refs[0].ID, dtp[0].Data.ID) - assert.Equal(t, "blob/1", dtp[0].Blob.PayloadRef) + assert.Len(t, newMsg.ResolvedData.AllData, 1) + assert.Empty(t, newMsg.ResolvedData.NewData) + assert.Len(t, newMsg.ResolvedData.DataToPublish, 1) + assert.Equal(t, dataID, newMsg.ResolvedData.AllData[0].ID) + assert.Equal(t, dataHash, newMsg.ResolvedData.AllData[0].Hash) + assert.Len(t, newMsg.ResolvedData.DataToPublish, 1) + assert.Equal(t, newMsg.ResolvedData.AllData[0].ID, newMsg.ResolvedData.DataToPublish[0].Data.ID) + assert.Equal(t, "blob/1", newMsg.ResolvedData.DataToPublish[0].Blob.PayloadRef) } func TestResolveInlineDataBroadcastResolveBlobFail(t *testing.T) { @@ -334,8 +458,7 @@ func TestResolveInlineDataBroadcastResolveBlobFail(t *testing.T) { defer cancel() mdi := dm.database.(*databasemocks.Plugin) - dataID := fftypes.NewUUID() - dataHash := fftypes.NewRandB32() + dataID, dataHash, newMsg := testNewMessage() blobHash := fftypes.NewRandB32() mdi.On("GetDataByID", ctx, dataID, true).Return(&fftypes.Data{ @@ -348,9 +471,7 @@ func TestResolveInlineDataBroadcastResolveBlobFail(t *testing.T) { }, nil) mdi.On("GetBlobMatchingHash", ctx, blobHash).Return(nil, fmt.Errorf("pop")) - _, _, err := dm.ResolveInlineDataBroadcast(ctx, "ns1", fftypes.InlineData{ - {DataRef: fftypes.DataRef{ID: dataID}}, - }) + err := dm.ResolveInlineDataBroadcast(ctx, newMsg) assert.EqualError(t, err, "pop") } @@ -359,8 +480,7 @@ func TestResolveInlineDataRefBadNamespace(t *testing.T) { defer cancel() mdi := dm.database.(*databasemocks.Plugin) - dataID := fftypes.NewUUID() - dataHash := fftypes.NewRandB32() + dataID, dataHash, newMsg := testNewMessage() mdi.On("GetDataByID", ctx, dataID, true).Return(&fftypes.Data{ ID: dataID, @@ -368,11 +488,8 @@ func TestResolveInlineDataRefBadNamespace(t *testing.T) { Hash: dataHash, }, nil) - data, err := dm.ResolveInlineDataPrivate(ctx, "ns1", fftypes.InlineData{ - {DataRef: fftypes.DataRef{ID: dataID, Hash: dataHash}}, - }) + err := dm.ResolveInlineDataPrivate(ctx, newMsg) assert.Regexp(t, "FF10204", err) - assert.Empty(t, data) } func TestResolveInlineDataRefBadHash(t *testing.T) { @@ -380,8 +497,7 @@ func TestResolveInlineDataRefBadHash(t *testing.T) { defer cancel() mdi := dm.database.(*databasemocks.Plugin) - dataID := fftypes.NewUUID() - dataHash := fftypes.NewRandB32() + dataID, dataHash, newMsg := testNewMessage() mdi.On("GetDataByID", ctx, dataID, true).Return(&fftypes.Data{ ID: dataID, @@ -389,11 +505,8 @@ func TestResolveInlineDataRefBadHash(t *testing.T) { Hash: dataHash, }, nil) - data, err := dm.ResolveInlineDataPrivate(ctx, "ns1", fftypes.InlineData{ - {DataRef: fftypes.DataRef{ID: dataID, Hash: fftypes.NewRandB32()}}, - }) + err := dm.ResolveInlineDataPrivate(ctx, newMsg) assert.Regexp(t, "FF10204", err) - assert.Empty(t, data) } func TestResolveInlineDataRefLookkupFail(t *testing.T) { @@ -401,13 +514,11 @@ func TestResolveInlineDataRefLookkupFail(t *testing.T) { defer cancel() mdi := dm.database.(*databasemocks.Plugin) - dataID := fftypes.NewUUID() + dataID, _, newMsg := testNewMessage() mdi.On("GetDataByID", ctx, dataID, true).Return(nil, fmt.Errorf("pop")) - _, err := dm.ResolveInlineDataPrivate(ctx, "ns1", fftypes.InlineData{ - {DataRef: fftypes.DataRef{ID: dataID, Hash: fftypes.NewRandB32()}}, - }) + err := dm.ResolveInlineDataPrivate(ctx, newMsg) assert.EqualError(t, err, "pop") } @@ -418,26 +529,17 @@ func TestResolveInlineDataValueNoValidatorOK(t *testing.T) { mdi.On("UpsertData", ctx, mock.Anything, database.UpsertOptimizationNew).Return(nil) - data, err := dm.ResolveInlineDataPrivate(ctx, "ns1", fftypes.InlineData{ + _, _, newMsg := testNewMessage() + newMsg.InData = fftypes.InlineData{ {Value: fftypes.JSONAnyPtr(`{"some":"json"}`)}, - }) - assert.NoError(t, err) - assert.Len(t, data, 1) - assert.NotNil(t, data[0].ID) - assert.NotNil(t, data[0].Hash) -} - -func TestResolveInlineDataValueNoValidatorStoreFail(t *testing.T) { - dm, ctx, cancel := newTestDataManager(t) - defer cancel() - mdi := dm.database.(*databasemocks.Plugin) - - mdi.On("UpsertData", ctx, mock.Anything, database.UpsertOptimizationNew).Return(fmt.Errorf("pop")) + } - _, err := dm.ResolveInlineDataPrivate(ctx, "ns1", fftypes.InlineData{ - {Value: fftypes.JSONAnyPtr(`{"some":"json"}`)}, - }) - assert.EqualError(t, err, "pop") + err := dm.ResolveInlineDataPrivate(ctx, newMsg) + assert.NoError(t, err) + assert.Len(t, newMsg.ResolvedData.AllData, 1) + assert.Len(t, newMsg.ResolvedData.NewData, 1) + assert.NotNil(t, newMsg.ResolvedData.AllData[0].ID) + assert.NotNil(t, newMsg.ResolvedData.AllData[0].Hash) } func TestResolveInlineDataValueWithValidation(t *testing.T) { @@ -462,7 +564,8 @@ func TestResolveInlineDataValueWithValidation(t *testing.T) { }`), }, nil) - data, err := dm.ResolveInlineDataPrivate(ctx, "ns1", fftypes.InlineData{ + _, _, newMsg := testNewMessage() + newMsg.InData = fftypes.InlineData{ { Datatype: &fftypes.DatatypeRef{ Name: "customer", @@ -470,13 +573,16 @@ func TestResolveInlineDataValueWithValidation(t *testing.T) { }, Value: fftypes.JSONAnyPtr(`{"field1":"value1"}`), }, - }) + } + + err := dm.ResolveInlineDataPrivate(ctx, newMsg) assert.NoError(t, err) - assert.Len(t, data, 1) - assert.NotNil(t, data[0].ID) - assert.NotNil(t, data[0].Hash) + assert.Len(t, newMsg.ResolvedData.AllData, 1) + assert.Len(t, newMsg.ResolvedData.NewData, 1) + assert.NotNil(t, newMsg.ResolvedData.AllData[0].ID) + assert.NotNil(t, newMsg.ResolvedData.AllData[0].Hash) - _, err = dm.ResolveInlineDataPrivate(ctx, "ns1", fftypes.InlineData{ + newMsg.InData = fftypes.InlineData{ { Datatype: &fftypes.DatatypeRef{ Name: "customer", @@ -484,7 +590,8 @@ func TestResolveInlineDataValueWithValidation(t *testing.T) { }, Value: fftypes.JSONAnyPtr(`{"not_allowed":"value"}`), }, - }) + } + err = dm.ResolveInlineDataPrivate(ctx, newMsg) assert.Regexp(t, "FF10198", err) } @@ -492,9 +599,12 @@ func TestResolveInlineDataNoRefOrValue(t *testing.T) { dm, ctx, cancel := newTestDataManager(t) defer cancel() - _, err := dm.ResolveInlineDataPrivate(ctx, "ns1", fftypes.InlineData{ + _, _, newMsg := testNewMessage() + newMsg.InData = fftypes.InlineData{ { /* missing */ }, - }) + } + + err := dm.ResolveInlineDataPrivate(ctx, newMsg) assert.Regexp(t, "FF10205", err) } @@ -513,11 +623,21 @@ func TestUploadJSONLoadDatatypeFail(t *testing.T) { assert.EqualError(t, err, "pop") } +func TestUploadJSONLoadInsertDataFail(t *testing.T) { + dm, ctx, cancel := newTestDataManager(t) + defer cancel() + dm.messageWriter.close() + _, err := dm.UploadJSON(ctx, "ns1", &fftypes.DataRefOrValue{ + Value: fftypes.JSONAnyPtr(`{}`), + }) + assert.Regexp(t, "FF10158", err) +} + func TestValidateAndStoreLoadNilRef(t *testing.T) { dm, ctx, cancel := newTestDataManager(t) defer cancel() - _, _, err := dm.validateAndStoreInlined(ctx, "ns1", &fftypes.DataRefOrValue{ + _, _, err := dm.validateInputData(ctx, "ns1", &fftypes.DataRefOrValue{ Validator: fftypes.ValidatorTypeJSON, Datatype: nil, }) @@ -530,7 +650,7 @@ func TestValidateAndStoreLoadValidatorUnknown(t *testing.T) { defer cancel() mdi := dm.database.(*databasemocks.Plugin) mdi.On("GetDatatypeByName", mock.Anything, "ns1", "customer", "0.0.1").Return(nil, nil) - _, _, err := dm.validateAndStoreInlined(ctx, "ns1", &fftypes.DataRefOrValue{ + _, _, err := dm.validateInputData(ctx, "ns1", &fftypes.DataRefOrValue{ Validator: "wrong!", Datatype: &fftypes.DatatypeRef{ Name: "customer", @@ -547,7 +667,7 @@ func TestValidateAndStoreLoadBadRef(t *testing.T) { defer cancel() mdi := dm.database.(*databasemocks.Plugin) mdi.On("GetDatatypeByName", mock.Anything, "ns1", "customer", "0.0.1").Return(nil, nil) - _, _, err := dm.validateAndStoreInlined(ctx, "ns1", &fftypes.DataRefOrValue{ + _, _, err := dm.validateInputData(ctx, "ns1", &fftypes.DataRefOrValue{ Datatype: &fftypes.DatatypeRef{ // Missing name }, @@ -561,7 +681,7 @@ func TestValidateAndStoreNotFound(t *testing.T) { defer cancel() mdi := dm.database.(*databasemocks.Plugin) mdi.On("GetDatatypeByName", mock.Anything, "ns1", "customer", "0.0.1").Return(nil, nil) - _, _, err := dm.validateAndStoreInlined(ctx, "ns1", &fftypes.DataRefOrValue{ + _, _, err := dm.validateInputData(ctx, "ns1", &fftypes.DataRefOrValue{ Datatype: &fftypes.DatatypeRef{ Name: "customer", Version: "0.0.1", @@ -577,7 +697,7 @@ func TestValidateAndStoreBlobError(t *testing.T) { mdi := dm.database.(*databasemocks.Plugin) blobHash := fftypes.NewRandB32() mdi.On("GetBlobMatchingHash", mock.Anything, blobHash).Return(nil, fmt.Errorf("pop")) - _, _, err := dm.validateAndStoreInlined(ctx, "ns1", &fftypes.DataRefOrValue{ + _, _, err := dm.validateInputData(ctx, "ns1", &fftypes.DataRefOrValue{ Blob: &fftypes.BlobRef{ Hash: blobHash, }, @@ -592,7 +712,7 @@ func TestValidateAndStoreBlobNotFound(t *testing.T) { mdi := dm.database.(*databasemocks.Plugin) blobHash := fftypes.NewRandB32() mdi.On("GetBlobMatchingHash", mock.Anything, blobHash).Return(nil, nil) - _, _, err := dm.validateAndStoreInlined(ctx, "ns1", &fftypes.DataRefOrValue{ + _, _, err := dm.validateInputData(ctx, "ns1", &fftypes.DataRefOrValue{ Blob: &fftypes.BlobRef{ Hash: blobHash, }, diff --git a/internal/data/message_writer.go b/internal/data/message_writer.go new file mode 100644 index 0000000000..e10862860f --- /dev/null +++ b/internal/data/message_writer.go @@ -0,0 +1,207 @@ +// Copyright © 2022 Kaleido, Inc. +// +// SPDX-License-Identifier: Apache-2.0 +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package data + +import ( + "context" + "time" + + "github.com/hyperledger/firefly/internal/i18n" + "github.com/hyperledger/firefly/internal/log" + "github.com/hyperledger/firefly/pkg/database" + "github.com/hyperledger/firefly/pkg/fftypes" +) + +type NewMessage struct { + Message *fftypes.Message + InData fftypes.InlineData + ResolvedData Resolved +} + +type Resolved struct { + AllData fftypes.DataArray + NewData fftypes.DataArray + DataToPublish []*fftypes.DataAndBlob +} + +// writeRequest is a combination of a message and a list of data that is new and needs to be +// inserted into the database. +type writeRequest struct { + newMessage *fftypes.Message + newData fftypes.DataArray + result chan error +} + +type messageWriterBatch struct { + messages []*fftypes.Message + data fftypes.DataArray + listeners []chan error + timeoutContext context.Context + timeoutCancel func() +} + +// messageWriter manages writing messages to the database. +// +// Where supported, it starts background workers to perform batch commits against the database, +// to allow high throughput insertion of messages + data. +// +// Multiple message writers can be started, to combine +// concurrency with batching and tune for maximum throughput. +type messageWriter struct { + ctx context.Context + cancelFunc func() + database database.Plugin + workQueue chan *writeRequest + workersDone []chan struct{} + conf *messageWriterConf + closed bool +} + +type messageWriterConf struct { + workerCount int + batchTimeout time.Duration + maxInserts int +} + +func newMessageWriter(ctx context.Context, di database.Plugin, conf *messageWriterConf) *messageWriter { + if !di.Capabilities().Concurrency { + log.L(ctx).Infof("Database plugin not configured for concurrency. Batched message writing disabled") + conf.workerCount = 0 + } + mw := &messageWriter{ + conf: conf, + database: di, + } + mw.ctx, mw.cancelFunc = context.WithCancel(ctx) + return mw +} + +// WriteNewMessage is the external interface, which depending on whether we have a non-zero +// worker count will dispatch the work to the pool and wait for it to complete on a background +// transaction, or just run it in-line on the context passed ini. +func (mw *messageWriter) WriteNewMessage(ctx context.Context, newMsg *NewMessage) error { + if mw.conf.workerCount > 0 { + // Dispatch to background worker + nmi := &writeRequest{ + newMessage: newMsg.Message, + newData: newMsg.ResolvedData.NewData, + result: make(chan error), + } + select { + case mw.workQueue <- nmi: + case <-mw.ctx.Done(): + return i18n.NewError(ctx, i18n.MsgContextCanceled) + } + return <-nmi.result + } + // Otherwise do it in-line on this context + return mw.writeMessages(ctx, []*fftypes.Message{newMsg.Message}, newMsg.ResolvedData.NewData) +} + +// WriteData writes a piece of data independently of a message +func (mw *messageWriter) WriteData(ctx context.Context, data *fftypes.Data) error { + if mw.conf.workerCount > 0 { + // Dispatch to background worker + nmi := &writeRequest{ + newData: fftypes.DataArray{data}, + result: make(chan error), + } + select { + case mw.workQueue <- nmi: + case <-mw.ctx.Done(): + return i18n.NewError(ctx, i18n.MsgContextCanceled) + } + return <-nmi.result + } + // Otherwise do it in-line on this context + return mw.database.UpsertData(ctx, data, database.UpsertOptimizationNew) +} + +func (mw *messageWriter) start() { + if mw.conf.workerCount > 0 { + mw.workQueue = make(chan *writeRequest) + mw.workersDone = make([]chan struct{}, mw.conf.workerCount) + for i := 0; i < mw.conf.workerCount; i++ { + mw.workersDone[i] = make(chan struct{}) + go mw.writerLoop(i) + } + } +} + +func (mw *messageWriter) writerLoop(index int) { + defer close(mw.workersDone[index]) + + var batch *messageWriterBatch + for !mw.closed { + var timeoutContext context.Context + var timedOut bool + if batch != nil { + timeoutContext = batch.timeoutContext + } else { + timeoutContext = mw.ctx + } + select { + case work := <-mw.workQueue: + if batch == nil { + batch = &messageWriterBatch{} + batch.timeoutContext, batch.timeoutCancel = context.WithTimeout(mw.ctx, mw.conf.batchTimeout) + } + if work.newMessage != nil { + batch.messages = append(batch.messages, work.newMessage) + } + batch.data = append(batch.data, work.newData...) + batch.listeners = append(batch.listeners, work.result) + case <-timeoutContext.Done(): + timedOut = true + } + + if batch != nil && (timedOut || (len(batch.messages)+len(batch.data) >= mw.conf.maxInserts)) { + batch.timeoutCancel() + err := mw.database.RunAsGroup(mw.ctx, func(ctx context.Context) error { + return mw.writeMessages(ctx, batch.messages, batch.data) + }) + for _, l := range batch.listeners { + l <- err + } + batch = nil + } + } +} + +func (mw *messageWriter) writeMessages(ctx context.Context, msgs []*fftypes.Message, data fftypes.DataArray) error { + if len(msgs) > 0 { + if err := mw.database.InsertMessages(ctx, msgs); err != nil { + return err + } + } + if len(data) > 0 { + if err := mw.database.InsertDataArray(ctx, data); err != nil { + return err + } + } + return nil +} + +func (mw *messageWriter) close() { + if !mw.closed { + mw.closed = true + mw.cancelFunc() + for _, workerDone := range mw.workersDone { + <-workerDone + } + } +} diff --git a/internal/data/message_writer_test.go b/internal/data/message_writer_test.go new file mode 100644 index 0000000000..e9c73df1cb --- /dev/null +++ b/internal/data/message_writer_test.go @@ -0,0 +1,135 @@ +// Copyright © 2021 Kaleido, Inc. +// +// SPDX-License-Identifier: Apache-2.0 +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package data + +import ( + "context" + "fmt" + "testing" + "time" + + "github.com/hyperledger/firefly/mocks/databasemocks" + "github.com/hyperledger/firefly/pkg/database" + "github.com/hyperledger/firefly/pkg/fftypes" + "github.com/stretchr/testify/assert" +) + +func newTestMessageWriter(t *testing.T) *messageWriter { + return newTestMessageWriterConf(t, &messageWriterConf{ + workerCount: 1, + batchTimeout: 100 * time.Millisecond, + maxInserts: 200, + }, &database.Capabilities{Concurrency: true}) +} + +func newTestMessageWriterNoConcrrency(t *testing.T) *messageWriter { + return newTestMessageWriterConf(t, &messageWriterConf{workerCount: 1}, &database.Capabilities{Concurrency: false}) +} + +func newTestMessageWriterConf(t *testing.T, conf *messageWriterConf, dbCapabilities *database.Capabilities) *messageWriter { + mdi := &databasemocks.Plugin{} + mdi.On("Capabilities").Return(dbCapabilities) + return newMessageWriter(context.Background(), mdi, conf) +} + +func TestNewMessageWriterNoConcurrency(t *testing.T) { + mw := newTestMessageWriterNoConcrrency(t) + assert.Zero(t, mw.conf.workerCount) +} + +func TestWriteNewMessageClosed(t *testing.T) { + mw := newTestMessageWriter(t) + mw.close() + err := mw.WriteNewMessage(mw.ctx, &NewMessage{}) + assert.Regexp(t, "FF10158", err) +} + +func TestWriteDataClosed(t *testing.T) { + mw := newTestMessageWriter(t) + mw.close() + err := mw.WriteData(mw.ctx, &fftypes.Data{}) + assert.Regexp(t, "FF10158", err) +} + +func TestWriteNewMessageSyncFallback(t *testing.T) { + mw := newTestMessageWriterNoConcrrency(t) + customCtx := context.WithValue(context.Background(), "dbtx", "on this context") + + msg1 := &fftypes.Message{ + Header: fftypes.MessageHeader{ + ID: fftypes.NewUUID(), + }, + } + data1 := &fftypes.Data{ID: fftypes.NewUUID()} + + mdi := mw.database.(*databasemocks.Plugin) + mdi.On("InsertMessages", customCtx, []*fftypes.Message{msg1}).Return(nil) + mdi.On("InsertDataArray", customCtx, fftypes.DataArray{data1}).Return(nil) + + err := mw.WriteNewMessage(customCtx, &NewMessage{ + Message: msg1, + ResolvedData: Resolved{ + NewData: fftypes.DataArray{data1}, + }, + }) + + assert.NoError(t, err) +} + +func TestWriteDataSyncFallback(t *testing.T) { + mw := newTestMessageWriterNoConcrrency(t) + customCtx := context.WithValue(context.Background(), "dbtx", "on this context") + + data1 := &fftypes.Data{ID: fftypes.NewUUID()} + + mdi := mw.database.(*databasemocks.Plugin) + mdi.On("UpsertData", customCtx, data1, database.UpsertOptimizationNew).Return(nil) + + err := mw.WriteData(customCtx, data1) + + assert.NoError(t, err) +} + +func TestWriteMessagesInsertMessagesFail(t *testing.T) { + mw := newTestMessageWriterNoConcrrency(t) + + msg1 := &fftypes.Message{ + Header: fftypes.MessageHeader{ + ID: fftypes.NewUUID(), + }, + } + + mdi := mw.database.(*databasemocks.Plugin) + mdi.On("InsertMessages", mw.ctx, []*fftypes.Message{msg1}).Return(fmt.Errorf("pop")) + + err := mw.writeMessages(mw.ctx, []*fftypes.Message{msg1}, fftypes.DataArray{}) + + assert.Regexp(t, "pop", err) +} + +func TestWriteMessagesInsertDataArrayFail(t *testing.T) { + mw := newTestMessageWriterNoConcrrency(t) + + data1 := &fftypes.Data{ID: fftypes.NewUUID()} + + mdi := mw.database.(*databasemocks.Plugin) + mdi.On("InsertDataArray", mw.ctx, fftypes.DataArray{data1}).Return(fmt.Errorf("pop")) + + err := mw.writeMessages(mw.ctx, []*fftypes.Message{}, fftypes.DataArray{data1}) + + assert.Regexp(t, "pop", err) +} diff --git a/internal/database/sqlcommon/data_sql.go b/internal/database/sqlcommon/data_sql.go index cb18d2c483..352e5af3b1 100644 --- a/internal/database/sqlcommon/data_sql.go +++ b/internal/database/sqlcommon/data_sql.go @@ -54,7 +54,15 @@ var ( } ) -func (s *SQLCommon) attemptDataUpdate(ctx context.Context, tx *txWrapper, data *fftypes.Data, datatype *fftypes.DatatypeRef, blob *fftypes.BlobRef) (int64, error) { +func (s *SQLCommon) attemptDataUpdate(ctx context.Context, tx *txWrapper, data *fftypes.Data) (int64, error) { + datatype := data.Datatype + if datatype == nil { + datatype = &fftypes.DatatypeRef{} + } + blob := data.Blob + if blob == nil { + blob = &fftypes.BlobRef{} + } data.ValueSize = data.Value.Length() return s.updateTx(ctx, tx, sq.Update("data"). @@ -79,26 +87,36 @@ func (s *SQLCommon) attemptDataUpdate(ctx context.Context, tx *txWrapper, data * }) } -func (s *SQLCommon) attemptDataInsert(ctx context.Context, tx *txWrapper, data *fftypes.Data, datatype *fftypes.DatatypeRef, blob *fftypes.BlobRef, requestConflictEmptyResult bool) (int64, error) { +func (s *SQLCommon) setDataInsertValues(query sq.InsertBuilder, data *fftypes.Data) sq.InsertBuilder { + datatype := data.Datatype + if datatype == nil { + datatype = &fftypes.DatatypeRef{} + } + blob := data.Blob + if blob == nil { + blob = &fftypes.BlobRef{} + } + return query.Values( + data.ID, + string(data.Validator), + data.Namespace, + datatype.Name, + datatype.Version, + data.Hash, + data.Created, + blob.Hash, + blob.Public, + blob.Name, + blob.Size, + data.ValueSize, + data.Value, + ) +} + +func (s *SQLCommon) attemptDataInsert(ctx context.Context, tx *txWrapper, data *fftypes.Data, requestConflictEmptyResult bool) (int64, error) { data.ValueSize = data.Value.Length() return s.insertTxExt(ctx, tx, - sq.Insert("data"). - Columns(dataColumnsWithValue...). - Values( - data.ID, - string(data.Validator), - data.Namespace, - datatype.Name, - datatype.Version, - data.Hash, - data.Created, - blob.Hash, - blob.Public, - blob.Name, - blob.Size, - data.ValueSize, - data.Value, - ), + s.setDataInsertValues(sq.Insert("data").Columns(dataColumnsWithValue...), data), func() { s.callbacks.UUIDCollectionNSEvent(database.CollectionData, fftypes.ChangeEventTypeCreated, data.Namespace, data.ID) }, requestConflictEmptyResult) @@ -111,15 +129,6 @@ func (s *SQLCommon) UpsertData(ctx context.Context, data *fftypes.Data, optimiza } defer s.rollbackTx(ctx, tx, autoCommit) - datatype := data.Datatype - if datatype == nil { - datatype = &fftypes.DatatypeRef{} - } - blob := data.Blob - if blob == nil { - blob = &fftypes.BlobRef{} - } - // This is a performance critical function, as we stream data into the database for every message, in every batch. // // First attempt the operation based on the optimization passed in. @@ -127,10 +136,10 @@ func (s *SQLCommon) UpsertData(ctx context.Context, data *fftypes.Data, optimiza // as only recovery paths require us to go down the un-optimized route. optimized := false if optimization == database.UpsertOptimizationNew { - _, opErr := s.attemptDataInsert(ctx, tx, data, datatype, blob, true /* we want a failure here we can progress past */) + _, opErr := s.attemptDataInsert(ctx, tx, data, true /* we want a failure here we can progress past */) optimized = opErr == nil } else if optimization == database.UpsertOptimizationExisting { - rowsAffected, opErr := s.attemptDataUpdate(ctx, tx, data, datatype, blob) + rowsAffected, opErr := s.attemptDataUpdate(ctx, tx, data) optimized = opErr == nil && rowsAffected == 1 } @@ -158,11 +167,11 @@ func (s *SQLCommon) UpsertData(ctx context.Context, data *fftypes.Data, optimiza dataRows.Close() if existing { - if _, err = s.attemptDataUpdate(ctx, tx, data, datatype, blob); err != nil { + if _, err = s.attemptDataUpdate(ctx, tx, data); err != nil { return err } } else { - if _, err = s.attemptDataInsert(ctx, tx, data, datatype, blob, false); err != nil { + if _, err = s.attemptDataInsert(ctx, tx, data, false); err != nil { return err } } @@ -171,6 +180,42 @@ func (s *SQLCommon) UpsertData(ctx context.Context, data *fftypes.Data, optimiza return s.commitTx(ctx, tx, autoCommit) } +func (s *SQLCommon) InsertDataArray(ctx context.Context, dataArray fftypes.DataArray) (err error) { + + ctx, tx, autoCommit, err := s.beginOrUseTx(ctx) + if err != nil { + return err + } + defer s.rollbackTx(ctx, tx, autoCommit) + + if s.features.MultiRowInsert { + query := sq.Insert("data").Columns(dataColumnsWithValue...) + for _, data := range dataArray { + query = s.setDataInsertValues(query, data) + } + sequences := make([]int64, len(dataArray)) + err := s.insertTxRows(ctx, tx, query, func() { + for _, data := range dataArray { + s.callbacks.UUIDCollectionNSEvent(database.CollectionData, fftypes.ChangeEventTypeCreated, data.Namespace, data.ID) + } + }, sequences, false) + if err != nil { + return err + } + } else { + // Fall back to individual inserts grouped in a TX + for _, data := range dataArray { + _, err := s.attemptDataInsert(ctx, tx, data, false) + if err != nil { + return err + } + } + } + + return s.commitTx(ctx, tx, autoCommit) + +} + func (s *SQLCommon) dataResult(ctx context.Context, row *sql.Rows, withValue bool) (*fftypes.Data, error) { data := fftypes.Data{ Datatype: &fftypes.DatatypeRef{}, diff --git a/internal/database/sqlcommon/data_sql_test.go b/internal/database/sqlcommon/data_sql_test.go index 82c1e45b16..c76963706c 100644 --- a/internal/database/sqlcommon/data_sql_test.go +++ b/internal/database/sqlcommon/data_sql_test.go @@ -222,6 +222,61 @@ func TestUpsertDataFailCommit(t *testing.T) { assert.NoError(t, mock.ExpectationsWereMet()) } +func TestInsertDataArrayBeginFail(t *testing.T) { + s, mock := newMockProvider().init() + mock.ExpectBegin().WillReturnError(fmt.Errorf("pop")) + err := s.InsertDataArray(context.Background(), fftypes.DataArray{}) + assert.Regexp(t, "FF10114", err) + assert.NoError(t, mock.ExpectationsWereMet()) + s.callbacks.AssertExpectations(t) +} + +func TestInsertDataArrayMultiRowOK(t *testing.T) { + s, mock := newMockProvider().init() + s.features.MultiRowInsert = true + s.fakePSQLInsert = true + + data1 := &fftypes.Data{ID: fftypes.NewUUID(), Namespace: "ns1"} + data2 := &fftypes.Data{ID: fftypes.NewUUID(), Namespace: "ns1"} + s.callbacks.On("UUIDCollectionNSEvent", database.CollectionData, fftypes.ChangeEventTypeCreated, "ns1", data1.ID) + s.callbacks.On("UUIDCollectionNSEvent", database.CollectionData, fftypes.ChangeEventTypeCreated, "ns1", data2.ID) + + mock.ExpectBegin() + mock.ExpectQuery("INSERT.*").WillReturnRows(sqlmock.NewRows([]string{sequenceColumn}). + AddRow(int64(1001)). + AddRow(int64(1002)), + ) + mock.ExpectCommit() + err := s.InsertDataArray(context.Background(), fftypes.DataArray{data1, data2}) + assert.NoError(t, err) + assert.NoError(t, mock.ExpectationsWereMet()) + s.callbacks.AssertExpectations(t) +} + +func TestInsertDataArrayMultiRowFail(t *testing.T) { + s, mock := newMockProvider().init() + s.features.MultiRowInsert = true + s.fakePSQLInsert = true + data1 := &fftypes.Data{ID: fftypes.NewUUID(), Namespace: "ns1"} + mock.ExpectBegin() + mock.ExpectQuery("INSERT.*").WillReturnError(fmt.Errorf("pop")) + err := s.InsertDataArray(context.Background(), fftypes.DataArray{data1}) + assert.Regexp(t, "FF10116", err) + assert.NoError(t, mock.ExpectationsWereMet()) + s.callbacks.AssertExpectations(t) +} + +func TestInsertDataArraySingleRowFail(t *testing.T) { + s, mock := newMockProvider().init() + data1 := &fftypes.Data{ID: fftypes.NewUUID(), Namespace: "ns1"} + mock.ExpectBegin() + mock.ExpectExec("INSERT.*").WillReturnError(fmt.Errorf("pop")) + err := s.InsertDataArray(context.Background(), fftypes.DataArray{data1}) + assert.Regexp(t, "FF10116", err) + assert.NoError(t, mock.ExpectationsWereMet()) + s.callbacks.AssertExpectations(t) +} + func TestGetDataByIDSelectFail(t *testing.T) { s, mock := newMockProvider().init() dataID := fftypes.NewUUID() diff --git a/internal/database/sqlcommon/message_sql.go b/internal/database/sqlcommon/message_sql.go index 303aa22ef6..87a6449a17 100644 --- a/internal/database/sqlcommon/message_sql.go +++ b/internal/database/sqlcommon/message_sql.go @@ -84,29 +84,31 @@ func (s *SQLCommon) attemptMessageUpdate(ctx context.Context, tx *txWrapper, mes }) } +func (s *SQLCommon) setMessageInsertValues(query sq.InsertBuilder, message *fftypes.Message) sq.InsertBuilder { + return query.Values( + message.Header.ID, + message.Header.CID, + string(message.Header.Type), + message.Header.Author, + message.Header.Key, + message.Header.Created, + message.Header.Namespace, + message.Header.Topics, + message.Header.Tag, + message.Header.Group, + message.Header.DataHash, + message.Hash, + message.Pins, + message.State, + message.Confirmed, + message.Header.TxType, + message.BatchID, + ) +} + func (s *SQLCommon) attemptMessageInsert(ctx context.Context, tx *txWrapper, message *fftypes.Message, requestConflictEmptyResult bool) (err error) { message.Sequence, err = s.insertTxExt(ctx, tx, - sq.Insert("messages"). - Columns(msgColumns...). - Values( - message.Header.ID, - message.Header.CID, - string(message.Header.Type), - message.Header.Author, - message.Header.Key, - message.Header.Created, - message.Header.Namespace, - message.Header.Topics, - message.Header.Tag, - message.Header.Group, - message.Header.DataHash, - message.Hash, - message.Pins, - message.State, - message.Confirmed, - message.Header.TxType, - message.BatchID, - ), + s.setMessageInsertValues(sq.Insert("messages").Columns(msgColumns...), message), func() { s.callbacks.OrderedUUIDCollectionNSEvent(database.CollectionMessages, fftypes.ChangeEventTypeCreated, message.Header.Namespace, message.Header.ID, message.Sequence) }, requestConflictEmptyResult) @@ -183,6 +185,43 @@ func (s *SQLCommon) UpsertMessage(ctx context.Context, message *fftypes.Message, return s.commitTx(ctx, tx, autoCommit) } +func (s *SQLCommon) InsertMessages(ctx context.Context, messages []*fftypes.Message) (err error) { + + ctx, tx, autoCommit, err := s.beginOrUseTx(ctx) + if err != nil { + return err + } + defer s.rollbackTx(ctx, tx, autoCommit) + + if s.features.MultiRowInsert { + query := sq.Insert("messages").Columns(msgColumns...) + for _, message := range messages { + query = s.setMessageInsertValues(query, message) + } + sequences := make([]int64, len(messages)) + err := s.insertTxRows(ctx, tx, query, func() { + for i, message := range messages { + message.Sequence = sequences[i] + s.callbacks.OrderedUUIDCollectionNSEvent(database.CollectionMessages, fftypes.ChangeEventTypeCreated, message.Header.Namespace, message.Header.ID, message.Sequence) + } + }, sequences, false) + if err != nil { + return err + } + } else { + // Fall back to individual inserts grouped in a TX + for _, message := range messages { + err := s.attemptMessageInsert(ctx, tx, message, false) + if err != nil { + return err + } + } + } + + return s.commitTx(ctx, tx, autoCommit) + +} + // In SQL update+bump is a delete+insert within a TX func (s *SQLCommon) ReplaceMessage(ctx context.Context, message *fftypes.Message) (err error) { ctx, tx, autoCommit, err := s.beginOrUseTx(ctx) diff --git a/internal/database/sqlcommon/message_sql_test.go b/internal/database/sqlcommon/message_sql_test.go index 1bb8bf5dbd..9145268806 100644 --- a/internal/database/sqlcommon/message_sql_test.go +++ b/internal/database/sqlcommon/message_sql_test.go @@ -276,6 +276,61 @@ func TestUpsertMessageFailCommit(t *testing.T) { assert.NoError(t, mock.ExpectationsWereMet()) } +func TestInsertMessagesBeginFail(t *testing.T) { + s, mock := newMockProvider().init() + mock.ExpectBegin().WillReturnError(fmt.Errorf("pop")) + err := s.InsertMessages(context.Background(), []*fftypes.Message{}) + assert.Regexp(t, "FF10114", err) + assert.NoError(t, mock.ExpectationsWereMet()) + s.callbacks.AssertExpectations(t) +} + +func TestInsertMessagesMultiRowOK(t *testing.T) { + s, mock := newMockProvider().init() + s.features.MultiRowInsert = true + s.fakePSQLInsert = true + + msg1 := &fftypes.Message{Header: fftypes.MessageHeader{ID: fftypes.NewUUID(), Namespace: "ns1"}} + msg2 := &fftypes.Message{Header: fftypes.MessageHeader{ID: fftypes.NewUUID(), Namespace: "ns1"}} + s.callbacks.On("OrderedUUIDCollectionNSEvent", database.CollectionMessages, fftypes.ChangeEventTypeCreated, "ns1", msg1.Header.ID, int64(1001)) + s.callbacks.On("OrderedUUIDCollectionNSEvent", database.CollectionMessages, fftypes.ChangeEventTypeCreated, "ns1", msg2.Header.ID, int64(1002)) + + mock.ExpectBegin() + mock.ExpectQuery("INSERT.*").WillReturnRows(sqlmock.NewRows([]string{sequenceColumn}). + AddRow(int64(1001)). + AddRow(int64(1002)), + ) + mock.ExpectCommit() + err := s.InsertMessages(context.Background(), []*fftypes.Message{msg1, msg2}) + assert.NoError(t, err) + assert.NoError(t, mock.ExpectationsWereMet()) + s.callbacks.AssertExpectations(t) +} + +func TestInsertMessagesMultiRowFail(t *testing.T) { + s, mock := newMockProvider().init() + s.features.MultiRowInsert = true + s.fakePSQLInsert = true + msg1 := &fftypes.Message{Header: fftypes.MessageHeader{ID: fftypes.NewUUID(), Namespace: "ns1"}} + mock.ExpectBegin() + mock.ExpectQuery("INSERT.*").WillReturnError(fmt.Errorf("pop")) + err := s.InsertMessages(context.Background(), []*fftypes.Message{msg1}) + assert.Regexp(t, "FF10116", err) + assert.NoError(t, mock.ExpectationsWereMet()) + s.callbacks.AssertExpectations(t) +} + +func TestInsertMessagesSingleRowFail(t *testing.T) { + s, mock := newMockProvider().init() + msg1 := &fftypes.Message{Header: fftypes.MessageHeader{ID: fftypes.NewUUID(), Namespace: "ns1"}} + mock.ExpectBegin() + mock.ExpectExec("INSERT.*").WillReturnError(fmt.Errorf("pop")) + err := s.InsertMessages(context.Background(), []*fftypes.Message{msg1}) + assert.Regexp(t, "FF10116", err) + assert.NoError(t, mock.ExpectationsWereMet()) + s.callbacks.AssertExpectations(t) +} + func TestReplaceMessageFailBegin(t *testing.T) { s, mock := newMockProvider().init() mock.ExpectBegin().WillReturnError(fmt.Errorf("pop")) diff --git a/internal/database/sqlcommon/provider.go b/internal/database/sqlcommon/provider.go index 2cd033029f..e4aa381385 100644 --- a/internal/database/sqlcommon/provider.go +++ b/internal/database/sqlcommon/provider.go @@ -29,6 +29,7 @@ const ( type SQLFeatures struct { UseILIKE bool + MultiRowInsert bool PlaceholderFormat sq.PlaceholderFormat ExclusiveTableLockSQL func(table string) string } @@ -36,6 +37,7 @@ type SQLFeatures struct { func DefaultSQLProviderFeatures() SQLFeatures { return SQLFeatures{ UseILIKE: false, + MultiRowInsert: false, PlaceholderFormat: sq.Dollar, } } diff --git a/internal/database/sqlcommon/provider_mock_test.go b/internal/database/sqlcommon/provider_mock_test.go index 2a4c404968..f07417e836 100644 --- a/internal/database/sqlcommon/provider_mock_test.go +++ b/internal/database/sqlcommon/provider_mock_test.go @@ -46,10 +46,14 @@ type mockProvider struct { } func newMockProvider() *mockProvider { + config.Reset() mp := &mockProvider{ - prefix: config.NewPluginConfig("unittest.mockdb"), + capabilities: &database.Capabilities{}, + callbacks: &databasemocks.Callbacks{}, + prefix: config.NewPluginConfig("unittest.mockdb"), } mp.SQLCommon.InitPrefix(mp, mp.prefix) + mp.prefix.Set(SQLConfMaxConnections, 10) mp.mockDB, mp.mdb, _ = sqlmock.New() return mp } diff --git a/internal/database/sqlcommon/sqlcommon.go b/internal/database/sqlcommon/sqlcommon.go index 24fa5818cc..34598f48de 100644 --- a/internal/database/sqlcommon/sqlcommon.go +++ b/internal/database/sqlcommon/sqlcommon.go @@ -70,6 +70,9 @@ func (s *SQLCommon) Init(ctx context.Context, provider Provider, prefix config.P if connLimit > 0 { s.db.SetMaxOpenConns(connLimit) } + if connLimit > 1 { + capabilities.Concurrency = true + } if prefix.GetBool(SQLConfMigrationsAuto) { if err = s.applyDBMigrations(ctx, prefix, provider); err != nil { @@ -241,40 +244,58 @@ func (s *SQLCommon) insertTx(ctx context.Context, tx *txWrapper, q sq.InsertBuil } func (s *SQLCommon) insertTxExt(ctx context.Context, tx *txWrapper, q sq.InsertBuilder, postCommit func(), requestConflictEmptyResult bool) (int64, error) { + sequences := []int64{-1} + err := s.insertTxRows(ctx, tx, q, postCommit, sequences, requestConflictEmptyResult) + return sequences[0], err +} + +func (s *SQLCommon) insertTxRows(ctx context.Context, tx *txWrapper, q sq.InsertBuilder, postCommit func(), sequences []int64, requestConflictEmptyResult bool) error { l := log.L(ctx) q, useQuery := s.provider.ApplyInsertQueryCustomizations(q, requestConflictEmptyResult) sqlQuery, args, err := q.PlaceholderFormat(s.features.PlaceholderFormat).ToSql() if err != nil { - return -1, i18n.WrapError(ctx, err, i18n.MsgDBQueryBuildFailed) + return i18n.WrapError(ctx, err, i18n.MsgDBQueryBuildFailed) } l.Debugf(`SQL-> insert: %s`, sqlQuery) l.Tracef(`SQL-> insert args: %+v`, args) - var sequence int64 if useQuery { - err := tx.sqlTX.QueryRowContext(ctx, sqlQuery, args...).Scan(&sequence) + result, err := tx.sqlTX.QueryContext(ctx, sqlQuery, args...) + for i := 0; i < len(sequences) && err == nil; i++ { + if result.Next() { + err = result.Scan(&sequences[i]) + } else { + err = i18n.NewError(ctx, i18n.MsgDBNoSequence, i+1) + } + } + if result != nil { + result.Close() + } if err != nil { level := logrus.DebugLevel if !requestConflictEmptyResult { level = logrus.ErrorLevel } l.Logf(level, `SQL insert failed (conflictEmptyRequested=%t): %s sql=[ %s ]: %s`, requestConflictEmptyResult, err, sqlQuery, err) - return -1, i18n.WrapError(ctx, err, i18n.MsgDBInsertFailed) + return i18n.WrapError(ctx, err, i18n.MsgDBInsertFailed) } } else { + if len(sequences) > 1 { + return i18n.WrapError(ctx, err, i18n.MsgDBMultiRowConfigError) + } res, err := tx.sqlTX.ExecContext(ctx, sqlQuery, args...) if err != nil { l.Errorf(`SQL insert failed: %s sql=[ %s ]: %s`, err, sqlQuery, err) - return -1, i18n.WrapError(ctx, err, i18n.MsgDBInsertFailed) + return i18n.WrapError(ctx, err, i18n.MsgDBInsertFailed) } - sequence, _ = res.LastInsertId() + sequences[0], _ = res.LastInsertId() } - l.Debugf(`SQL<- inserted sequence=%d`, sequence) + l.Debugf(`SQL<- inserted sequences=%v`, sequences) if postCommit != nil { s.postCommitEvent(tx, postCommit) } - return sequence, nil + return nil } func (s *SQLCommon) deleteTx(ctx context.Context, tx *txWrapper, q sq.DeleteBuilder, postCommit func()) error { diff --git a/internal/database/sqlcommon/sqlcommon_test.go b/internal/database/sqlcommon/sqlcommon_test.go index da73481c22..cb219419d7 100644 --- a/internal/database/sqlcommon/sqlcommon_test.go +++ b/internal/database/sqlcommon/sqlcommon_test.go @@ -321,3 +321,26 @@ func TestDoubleLock(t *testing.T) { assert.NoError(t, err) assert.NoError(t, mdb.ExpectationsWereMet()) } + +func TestInsertTxRowsBadConfig(t *testing.T) { + s, mdb := newMockProvider().init() + mdb.ExpectBegin() + ctx, tx, _, err := s.beginOrUseTx(context.Background()) + assert.NoError(t, err) + s.fakePSQLInsert = false + sb := sq.Insert("table").Columns("col1").Values(("val1")) + err = s.insertTxRows(ctx, tx, sb, nil, []int64{1, 2}, false) + assert.Regexp(t, "FF10374", err) +} + +func TestInsertTxRowsIncompleteReturn(t *testing.T) { + s, mdb := newMockProvider().init() + mdb.ExpectBegin() + mdb.ExpectQuery("INSERT.*").WillReturnRows(sqlmock.NewRows([]string{sequenceColumn}).AddRow(int64(1001))) + ctx, tx, _, err := s.beginOrUseTx(context.Background()) + assert.NoError(t, err) + s.fakePSQLInsert = true + sb := sq.Insert("table").Columns("col1").Values(("val1")) + err = s.insertTxRows(ctx, tx, sb, nil, []int64{1, 2}, false) + assert.Regexp(t, "FF10116", err) +} diff --git a/internal/i18n/en_translations.go b/internal/i18n/en_translations.go index b5294e3015..696d10486a 100644 --- a/internal/i18n/en_translations.go +++ b/internal/i18n/en_translations.go @@ -290,4 +290,6 @@ var ( MsgOperationNotSupported = ffm("FF10371", "Operation not supported", 400) MsgFailedToRetrieve = ffm("FF10372", "Failed to retrieve %s %s", 500) MsgBlobMissingPublic = ffm("FF10373", "Blob for data %s missing public payload reference while flushing batch", 500) + MsgDBMultiRowConfigError = ffm("FF10374", "Database invalid configuration - using multi-row insert on DB plugin that does not support query syntax for input") + MsgDBNoSequence = ffm("FF10375", "Failed to retrieve sequence for insert row %d", 500) ) diff --git a/internal/orchestrator/orchestrator.go b/internal/orchestrator/orchestrator.go index 95d301e1ee..68fd59a841 100644 --- a/internal/orchestrator/orchestrator.go +++ b/internal/orchestrator/orchestrator.go @@ -249,6 +249,10 @@ func (or *orchestrator) WaitStop() { or.broadcast.WaitStop() or.broadcast = nil } + if or.data != nil { + or.data.Close() + or.data = nil + } or.started = false } diff --git a/mocks/databasemocks/plugin.go b/mocks/databasemocks/plugin.go index 7c25ec42a9..0bfbcd2189 100644 --- a/mocks/databasemocks/plugin.go +++ b/mocks/databasemocks/plugin.go @@ -2221,6 +2221,20 @@ func (_m *Plugin) InsertBlockchainEvent(ctx context.Context, event *fftypes.Bloc return r0 } +// InsertDataArray provides a mock function with given fields: ctx, data +func (_m *Plugin) InsertDataArray(ctx context.Context, data fftypes.DataArray) error { + ret := _m.Called(ctx, data) + + var r0 error + if rf, ok := ret.Get(0).(func(context.Context, fftypes.DataArray) error); ok { + r0 = rf(ctx, data) + } else { + r0 = ret.Error(0) + } + + return r0 +} + // InsertEvent provides a mock function with given fields: ctx, data func (_m *Plugin) InsertEvent(ctx context.Context, data *fftypes.Event) error { ret := _m.Called(ctx, data) @@ -2235,6 +2249,20 @@ func (_m *Plugin) InsertEvent(ctx context.Context, data *fftypes.Event) error { return r0 } +// InsertMessages provides a mock function with given fields: ctx, messages +func (_m *Plugin) InsertMessages(ctx context.Context, messages []*fftypes.Message) error { + ret := _m.Called(ctx, messages) + + var r0 error + if rf, ok := ret.Get(0).(func(context.Context, []*fftypes.Message) error); ok { + r0 = rf(ctx, messages) + } else { + r0 = ret.Error(0) + } + + return r0 +} + // InsertNextPin provides a mock function with given fields: ctx, nextpin func (_m *Plugin) InsertNextPin(ctx context.Context, nextpin *fftypes.NextPin) error { ret := _m.Called(ctx, nextpin) diff --git a/mocks/datamocks/manager.go b/mocks/datamocks/manager.go index dca27bbbef..d149849925 100644 --- a/mocks/datamocks/manager.go +++ b/mocks/datamocks/manager.go @@ -32,6 +32,11 @@ func (_m *Manager) CheckDatatype(ctx context.Context, ns string, datatype *fftyp return r0 } +// Close provides a mock function with given fields: +func (_m *Manager) Close() { + _m.Called() +} + // CopyBlobPStoDX provides a mock function with given fields: ctx, _a1 func (_m *Manager) CopyBlobPStoDX(ctx context.Context, _a1 *fftypes.Data) (*fftypes.Blob, error) { ret := _m.Called(ctx, _a1) diff --git a/pkg/database/plugin.go b/pkg/database/plugin.go index 2622853a44..427b224247 100644 --- a/pkg/database/plugin.go +++ b/pkg/database/plugin.go @@ -77,6 +77,9 @@ type iMessageCollection interface { // must match the hash of the record that is being inserted. UpsertMessage(ctx context.Context, message *fftypes.Message, optimization UpsertOptimization) (err error) + // InsertMessages performs a batch insert of messages assured to be new records + InsertMessages(ctx context.Context, messages []*fftypes.Message) (err error) + // UpdateMessage - Update message UpdateMessage(ctx context.Context, id *fftypes.UUID, update Update) (err error) @@ -103,6 +106,9 @@ type iDataCollection interface { // must match the hash of the record that is being inserted. UpsertData(ctx context.Context, data *fftypes.Data, optimization UpsertOptimization) (err error) + // InsertDataArray performs a batch insert of data assured to be new records + InsertDataArray(ctx context.Context, data fftypes.DataArray) (err error) + // UpdateData - Update data UpdateData(ctx context.Context, id *fftypes.UUID, update Update) (err error) @@ -659,7 +665,7 @@ type Callbacks interface { // Capabilities defines the capabilities a plugin can report as implementing or not type Capabilities struct { - ClusterEvents bool + Concurrency bool } // NamespaceQueryFactory filter fields for namespaces From 382f1df975c39fac7e39c0e6da4dbc18cb3b82ef Mon Sep 17 00:00:00 2001 From: Peter Broadhurst Date: Wed, 9 Mar 2022 22:07:12 -0500 Subject: [PATCH 02/11] Update broadcast code for new writer API Signed-off-by: Peter Broadhurst --- internal/broadcast/datatype_test.go | 36 ++--- internal/broadcast/definition.go | 30 ++--- internal/broadcast/definition_test.go | 23 ---- internal/broadcast/manager.go | 16 +-- internal/broadcast/manager_test.go | 134 ++++++++----------- internal/broadcast/message.go | 95 ++++++-------- internal/broadcast/message_test.go | 182 ++++++++++---------------- internal/broadcast/namespace_test.go | 4 +- internal/broadcast/tokenpool_test.go | 12 +- internal/data/data_manager.go | 8 +- internal/data/data_manager_test.go | 4 + mocks/datamocks/manager.go | 73 +++++------ 12 files changed, 237 insertions(+), 380 deletions(-) diff --git a/internal/broadcast/datatype_test.go b/internal/broadcast/datatype_test.go index fe9b35300d..b2d009c5b6 100644 --- a/internal/broadcast/datatype_test.go +++ b/internal/broadcast/datatype_test.go @@ -73,12 +73,11 @@ func TestBroadcastDatatypeBadValue(t *testing.T) { func TestBroadcastUpsertFail(t *testing.T) { bm, cancel := newTestBroadcast(t) defer cancel() - mdi := bm.database.(*databasemocks.Plugin) mdm := bm.data.(*datamocks.Manager) mim := bm.identity.(*identitymanagermocks.Manager) mim.On("ResolveInputSigningIdentity", mock.Anything, "ns1", mock.Anything).Return(nil) - mdi.On("UpsertData", mock.Anything, mock.Anything, database.UpsertOptimizationNew).Return(fmt.Errorf("pop")) + mdm.On("WriteNewMessage", mock.Anything, mock.Anything, mock.Anything).Return(fmt.Errorf("pop")) mdm.On("VerifyNamespaceExists", mock.Anything, "ns1").Return(nil) mdm.On("CheckDatatype", mock.Anything, "ns1", mock.Anything).Return(nil) @@ -89,6 +88,9 @@ func TestBroadcastUpsertFail(t *testing.T) { Value: fftypes.JSONAnyPtr(`{"some": "data"}`), }, false) assert.EqualError(t, err, "pop") + + mim.AssertExpectations(t) + mdm.AssertExpectations(t) } func TestBroadcastDatatypeInvalid(t *testing.T) { @@ -112,41 +114,16 @@ func TestBroadcastDatatypeInvalid(t *testing.T) { assert.EqualError(t, err, "pop") } -func TestBroadcastBroadcastFail(t *testing.T) { - bm, cancel := newTestBroadcast(t) - defer cancel() - mdi := bm.database.(*databasemocks.Plugin) - mdm := bm.data.(*datamocks.Manager) - mim := bm.identity.(*identitymanagermocks.Manager) - - mim.On("ResolveInputSigningIdentity", mock.Anything, "ns1", mock.Anything).Return(nil) - mdi.On("UpsertData", mock.Anything, mock.Anything, database.UpsertOptimizationNew).Return(nil) - mdm.On("VerifyNamespaceExists", mock.Anything, "ns1").Return(nil) - mdm.On("CheckDatatype", mock.Anything, "ns1", mock.Anything).Return(nil) - mdi.On("UpsertMessage", mock.Anything, mock.Anything, database.UpsertOptimizationNew).Return(fmt.Errorf("pop")) - - _, err := bm.BroadcastDatatype(context.Background(), "ns1", &fftypes.Datatype{ - Namespace: "ns1", - Name: "ent1", - Version: "0.0.1", - Value: fftypes.JSONAnyPtr(`{"some": "data"}`), - }, false) - assert.EqualError(t, err, "pop") -} - func TestBroadcastOk(t *testing.T) { bm, cancel := newTestBroadcast(t) defer cancel() - mdi := bm.database.(*databasemocks.Plugin) mdm := bm.data.(*datamocks.Manager) mim := bm.identity.(*identitymanagermocks.Manager) mim.On("ResolveInputSigningIdentity", mock.Anything, "ns1", mock.Anything).Return(nil) - mdi.On("UpsertData", mock.Anything, mock.Anything, database.UpsertOptimizationNew).Return(nil) mdm.On("VerifyNamespaceExists", mock.Anything, "ns1").Return(nil) mdm.On("CheckDatatype", mock.Anything, "ns1", mock.Anything).Return(nil) - mdm.On("UpdateMessageCache", mock.Anything, mock.Anything).Return() - mdi.On("UpsertMessage", mock.Anything, mock.Anything, database.UpsertOptimizationNew).Return(nil) + mdm.On("WriteNewMessage", mock.Anything, mock.Anything, mock.Anything).Return(nil) _, err := bm.BroadcastDatatype(context.Background(), "ns1", &fftypes.Datatype{ Namespace: "ns1", @@ -155,4 +132,7 @@ func TestBroadcastOk(t *testing.T) { Value: fftypes.JSONAnyPtr(`{"some": "data"}`), }, false) assert.NoError(t, err) + + mdm.AssertExpectations(t) + mim.AssertExpectations(t) } diff --git a/internal/broadcast/definition.go b/internal/broadcast/definition.go index f45434bc99..41e9ca1470 100644 --- a/internal/broadcast/definition.go +++ b/internal/broadcast/definition.go @@ -20,9 +20,9 @@ import ( "context" "encoding/json" + "github.com/hyperledger/firefly/internal/data" "github.com/hyperledger/firefly/internal/i18n" "github.com/hyperledger/firefly/internal/identity" - "github.com/hyperledger/firefly/pkg/database" "github.com/hyperledger/firefly/pkg/fftypes" ) @@ -52,10 +52,10 @@ func (bm *broadcastManager) BroadcastIdentityClaim(ctx context.Context, ns strin return bm.broadcastDefinitionCommon(ctx, ns, def, signingIdentity, tag, waitConfirm) } -func (bm *broadcastManager) broadcastDefinitionCommon(ctx context.Context, ns string, def fftypes.Definition, signingIdentity *fftypes.SignerRef, tag string, waitConfirm bool) (msg *fftypes.Message, err error) { +func (bm *broadcastManager) broadcastDefinitionCommon(ctx context.Context, ns string, def fftypes.Definition, signingIdentity *fftypes.SignerRef, tag string, waitConfirm bool) (*fftypes.Message, error) { // Serialize it into a data object, as a piece of data we can write to a message - data := &fftypes.Data{ + d := &fftypes.Data{ Validator: fftypes.ValidatorTypeSystemDefinition, ID: fftypes.NewUUID(), Namespace: ns, @@ -63,21 +63,16 @@ func (bm *broadcastManager) broadcastDefinitionCommon(ctx context.Context, ns st } b, err := json.Marshal(&def) if err == nil { - data.Value = fftypes.JSONAnyPtrBytes(b) - err = data.Seal(ctx, nil) + d.Value = fftypes.JSONAnyPtrBytes(b) + err = d.Seal(ctx, nil) } if err != nil { return nil, i18n.WrapError(ctx, err, i18n.MsgSerializationFailed) } - // Write as data to the local store - if err = bm.database.UpsertData(ctx, data, database.UpsertOptimizationNew); err != nil { - return nil, err - } - // Create a broadcast message referring to the data - in := &fftypes.MessageInOut{ - Message: fftypes.Message{ + newMsg := &data.NewMessage{ + Message: &fftypes.Message{ Header: fftypes.MessageHeader{ Namespace: ns, Type: fftypes.MessageTypeDefinition, @@ -86,9 +81,9 @@ func (bm *broadcastManager) broadcastDefinitionCommon(ctx context.Context, ns st Tag: tag, TxType: fftypes.TransactionTypeBatchPin, }, - Data: fftypes.DataRefs{ - {ID: data.ID, Hash: data.Hash}, - }, + }, + ResolvedData: data.Resolved{ + NewData: fftypes.DataArray{d}, }, } @@ -96,9 +91,8 @@ func (bm *broadcastManager) broadcastDefinitionCommon(ctx context.Context, ns st sender := broadcastSender{ mgr: bm, namespace: ns, - msg: in, + msg: newMsg, resolved: true, - data: fftypes.DataArray{data}, } sender.setDefaults() if waitConfirm { @@ -106,5 +100,5 @@ func (bm *broadcastManager) broadcastDefinitionCommon(ctx context.Context, ns st } else { err = sender.Send(ctx) } - return &in.Message, err + return newMsg.Message, err } diff --git a/internal/broadcast/definition_test.go b/internal/broadcast/definition_test.go index 6afeb24585..b0aaaebd10 100644 --- a/internal/broadcast/definition_test.go +++ b/internal/broadcast/definition_test.go @@ -21,10 +21,8 @@ import ( "testing" "github.com/hyperledger/firefly/internal/identity" - "github.com/hyperledger/firefly/mocks/databasemocks" "github.com/hyperledger/firefly/mocks/identitymanagermocks" "github.com/hyperledger/firefly/mocks/syncasyncmocks" - "github.com/hyperledger/firefly/pkg/database" "github.com/hyperledger/firefly/pkg/fftypes" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/mock" @@ -34,18 +32,15 @@ func TestBroadcastDefinitionAsNodeConfirm(t *testing.T) { bm, cancel := newTestBroadcast(t) defer cancel() - mdi := bm.database.(*databasemocks.Plugin) msa := bm.syncasync.(*syncasyncmocks.Bridge) mim := bm.identity.(*identitymanagermocks.Manager) - mdi.On("UpsertData", mock.Anything, mock.Anything, database.UpsertOptimizationNew).Return(nil) mim.On("ResolveInputSigningIdentity", mock.Anything, "ff_system", mock.Anything).Return(nil) msa.On("WaitForMessage", bm.ctx, "ff_system", mock.Anything, mock.Anything).Return(nil, fmt.Errorf("pop")) _, err := bm.BroadcastDefinitionAsNode(bm.ctx, fftypes.SystemNamespace, &fftypes.Namespace{}, fftypes.SystemTagDefineNamespace, true) assert.EqualError(t, err, "pop") - mdi.AssertExpectations(t) msa.AssertExpectations(t) mim.AssertExpectations(t) } @@ -54,11 +49,9 @@ func TestBroadcastIdentityClaim(t *testing.T) { bm, cancel := newTestBroadcast(t) defer cancel() - mdi := bm.database.(*databasemocks.Plugin) msa := bm.syncasync.(*syncasyncmocks.Bridge) mim := bm.identity.(*identitymanagermocks.Manager) - mdi.On("UpsertData", mock.Anything, mock.Anything, database.UpsertOptimizationNew).Return(nil) mim.On("NormalizeSigningKey", mock.Anything, "0x1234", identity.KeyNormalizationBlockchainPlugin).Return("", nil) msa.On("WaitForMessage", bm.ctx, "ff_system", mock.Anything, mock.Anything).Return(nil, fmt.Errorf("pop")) @@ -69,7 +62,6 @@ func TestBroadcastIdentityClaim(t *testing.T) { }, fftypes.SystemTagDefineNamespace, true) assert.EqualError(t, err, "pop") - mdi.AssertExpectations(t) msa.AssertExpectations(t) mim.AssertExpectations(t) } @@ -96,35 +88,20 @@ func TestBroadcastDatatypeDefinitionAsNodeConfirm(t *testing.T) { bm, cancel := newTestBroadcast(t) defer cancel() - mdi := bm.database.(*databasemocks.Plugin) msa := bm.syncasync.(*syncasyncmocks.Bridge) mim := bm.identity.(*identitymanagermocks.Manager) ns := "customNamespace" - mdi.On("UpsertData", mock.Anything, mock.Anything, database.UpsertOptimizationNew).Return(nil) mim.On("ResolveInputSigningIdentity", mock.Anything, ns, mock.Anything).Return(nil) msa.On("WaitForMessage", bm.ctx, ns, mock.Anything, mock.Anything).Return(nil, fmt.Errorf("pop")) _, err := bm.BroadcastDefinitionAsNode(bm.ctx, ns, &fftypes.Datatype{}, fftypes.SystemTagDefineNamespace, true) assert.EqualError(t, err, "pop") - mdi.AssertExpectations(t) msa.AssertExpectations(t) mim.AssertExpectations(t) } -func TestBroadcastDefinitionAsNodeUpsertFail(t *testing.T) { - bm, cancel := newTestBroadcast(t) - defer cancel() - - mdi := bm.database.(*databasemocks.Plugin) - mdi.On("UpsertData", mock.Anything, mock.Anything, database.UpsertOptimizationNew).Return(fmt.Errorf("pop")) - mim := bm.identity.(*identitymanagermocks.Manager) - mim.On("ResolveInputSigningIdentity", mock.Anything, fftypes.SystemNamespace, mock.Anything).Return(nil) - _, err := bm.BroadcastDefinitionAsNode(bm.ctx, fftypes.SystemNamespace, &fftypes.Namespace{}, fftypes.SystemTagDefineNamespace, false) - assert.Regexp(t, "pop", err) -} - func TestBroadcastDefinitionBadIdentity(t *testing.T) { bm, cancel := newTestBroadcast(t) defer cancel() diff --git a/internal/broadcast/manager.go b/internal/broadcast/manager.go index 64a284c62f..942211cf02 100644 --- a/internal/broadcast/manager.go +++ b/internal/broadcast/manager.go @@ -144,8 +144,8 @@ func (bm *broadcastManager) dispatchBatch(ctx context.Context, state *batch.Disp return bm.batchpin.SubmitPinnedBatch(ctx, &state.Persisted, state.Pins) } -func (bm *broadcastManager) publishBlobs(ctx context.Context, dataToPublish []*fftypes.DataAndBlob) error { - for _, d := range dataToPublish { +func (bm *broadcastManager) publishBlobs(ctx context.Context, newMsg *data.NewMessage) error { + for _, d := range newMsg.ResolvedData.DataToPublish { // Stream from the local data exchange ... reader, err := bm.exchange.DownloadBLOB(ctx, d.Blob.PayloadRef) if err != nil { @@ -154,19 +154,11 @@ func (bm *broadcastManager) publishBlobs(ctx context.Context, dataToPublish []*f defer reader.Close() // ... to the shared storage - sharedRef, err := bm.sharedstorage.PublishData(ctx, reader) - if err != nil { - return err - } - log.L(ctx).Infof("Published blob with hash '%s' for data '%s' to shared storage: '%s'", d.Blob.Hash, d.Data.ID, sharedRef) - - // Update the data in the database, with the shared reference. - // We do this independently for each piece of data - update := database.DataQueryFactory.NewUpdate(ctx).Set("blob.public", sharedRef) - err = bm.database.UpdateData(ctx, d.Data.ID, update) + d.Data.Blob.Public, err = bm.sharedstorage.PublishData(ctx, reader) if err != nil { return err } + log.L(ctx).Infof("Published blob with hash '%s' for data '%s' to shared storage: '%s'", d.Blob.Hash, d.Data.ID, d.Data.Blob.Public) } return nil diff --git a/internal/broadcast/manager_test.go b/internal/broadcast/manager_test.go index 828361ebc5..00867306f7 100644 --- a/internal/broadcast/manager_test.go +++ b/internal/broadcast/manager_test.go @@ -26,6 +26,7 @@ import ( "github.com/hyperledger/firefly/internal/batch" "github.com/hyperledger/firefly/internal/config" + "github.com/hyperledger/firefly/internal/data" "github.com/hyperledger/firefly/mocks/batchmocks" "github.com/hyperledger/firefly/mocks/batchpinmocks" "github.com/hyperledger/firefly/mocks/blockchainmocks" @@ -37,7 +38,6 @@ import ( "github.com/hyperledger/firefly/mocks/operationmocks" "github.com/hyperledger/firefly/mocks/sharedstoragemocks" "github.com/hyperledger/firefly/mocks/syncasyncmocks" - "github.com/hyperledger/firefly/pkg/database" "github.com/hyperledger/firefly/pkg/fftypes" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/mock" @@ -108,41 +108,58 @@ func TestBroadcastMessageGood(t *testing.T) { bm, cancel := newTestBroadcast(t) defer cancel() - msg := &fftypes.MessageInOut{} - mdi := bm.database.(*databasemocks.Plugin) - mdi.On("UpsertMessage", mock.Anything, &msg.Message, database.UpsertOptimizationNew).Return(nil) + dataID := fftypes.NewUUID() + dataHash := fftypes.NewRandB32() + newMsg := &data.NewMessage{ + Message: &fftypes.Message{ + Header: fftypes.MessageHeader{ + ID: fftypes.NewUUID(), + }, + Data: fftypes.DataRefs{ + {ID: dataID, Hash: dataHash}, + }, + }, + ResolvedData: data.Resolved{ + AllData: fftypes.DataArray{ + {ID: dataID, Hash: dataHash}, + }, + }, + } + mdm := bm.data.(*datamocks.Manager) - mdm.On("UpdateMessageCache", mock.Anything, mock.Anything).Return() + mdm.On("WriteNewMessage", mock.Anything, newMsg).Return(nil) broadcast := broadcastSender{ mgr: bm, - msg: msg, + msg: newMsg, } err := broadcast.sendInternal(context.Background(), methodSend) assert.NoError(t, err) bm.Start() bm.WaitStop() - mdi.AssertExpectations(t) + + mdm.AssertExpectations(t) } func TestBroadcastMessageBad(t *testing.T) { bm, cancel := newTestBroadcast(t) defer cancel() - dupID := fftypes.NewUUID() - msg := &fftypes.MessageInOut{ - Message: fftypes.Message{ + newMsg := &data.NewMessage{ + Message: &fftypes.Message{ + Header: fftypes.MessageHeader{ + ID: fftypes.NewUUID(), + }, Data: fftypes.DataRefs{ - {ID: dupID /* missing hash */}, + {ID: fftypes.NewUUID(), Hash: nil}, }, }, } - bm.database.(*databasemocks.Plugin).On("UpsertMessage", mock.Anything, msg, false).Return(nil) broadcast := broadcastSender{ mgr: bm, - msg: msg, + msg: newMsg, } err := broadcast.sendInternal(context.Background(), methodSend) assert.Regexp(t, "FF10144", err) @@ -255,51 +272,9 @@ func TestDispatchBatchSubmitBroadcastFail(t *testing.T) { mom.AssertExpectations(t) } -func TestPublishBlobsUpdateDataFail(t *testing.T) { - bm, cancel := newTestBroadcast(t) - defer cancel() - mdi := bm.database.(*databasemocks.Plugin) - mdx := bm.exchange.(*dataexchangemocks.Plugin) - mps := bm.sharedstorage.(*sharedstoragemocks.Plugin) - mim := bm.identity.(*identitymanagermocks.Manager) - - blobHash := fftypes.NewRandB32() - dataID := fftypes.NewUUID() - - ctx := context.Background() - mdx.On("DownloadBLOB", ctx, "blob/1").Return(ioutil.NopCloser(bytes.NewReader([]byte(`some data`))), nil) - mps.On("PublishData", ctx, mock.MatchedBy(func(reader io.ReadCloser) bool { - b, err := ioutil.ReadAll(reader) - assert.NoError(t, err) - assert.Equal(t, "some data", string(b)) - return true - })).Return("payload-ref", nil) - mdi.On("UpdateData", ctx, mock.Anything, mock.Anything).Return(fmt.Errorf("pop")) - mim.On("ResolveInputIdentity", ctx, mock.Anything).Return(nil) - - err := bm.publishBlobs(ctx, []*fftypes.DataAndBlob{ - { - Data: &fftypes.Data{ - ID: dataID, - Blob: &fftypes.BlobRef{ - Hash: blobHash, - }, - }, - Blob: &fftypes.Blob{ - Hash: blobHash, - PayloadRef: "blob/1", - }, - }, - }) - assert.EqualError(t, err, "pop") - - mdi.AssertExpectations(t) -} - func TestPublishBlobsPublishFail(t *testing.T) { bm, cancel := newTestBroadcast(t) defer cancel() - mdi := bm.database.(*databasemocks.Plugin) mdx := bm.exchange.(*dataexchangemocks.Plugin) mps := bm.sharedstorage.(*sharedstoragemocks.Plugin) mim := bm.identity.(*identitymanagermocks.Manager) @@ -317,23 +292,26 @@ func TestPublishBlobsPublishFail(t *testing.T) { })).Return("", fmt.Errorf("pop")) mim.On("ResolveInputIdentity", ctx, mock.Anything).Return(nil) - err := bm.publishBlobs(ctx, []*fftypes.DataAndBlob{ - { - Data: &fftypes.Data{ - ID: dataID, - Blob: &fftypes.BlobRef{ - Hash: blobHash, + err := bm.publishBlobs(ctx, &data.NewMessage{ + ResolvedData: data.Resolved{ + DataToPublish: []*fftypes.DataAndBlob{ + { + Data: &fftypes.Data{ + ID: dataID, + Blob: &fftypes.BlobRef{ + Hash: blobHash, + }, + }, + Blob: &fftypes.Blob{ + Hash: blobHash, + PayloadRef: "blob/1", + }, }, }, - Blob: &fftypes.Blob{ - Hash: blobHash, - PayloadRef: "blob/1", - }, }, }) assert.EqualError(t, err, "pop") - mdi.AssertExpectations(t) } func TestPublishBlobsDownloadFail(t *testing.T) { @@ -350,18 +328,22 @@ func TestPublishBlobsDownloadFail(t *testing.T) { mdx.On("DownloadBLOB", ctx, "blob/1").Return(nil, fmt.Errorf("pop")) mim.On("ResolveInputIdentity", ctx, mock.Anything).Return(nil) - err := bm.publishBlobs(ctx, []*fftypes.DataAndBlob{ - { - Data: &fftypes.Data{ - ID: dataID, - Blob: &fftypes.BlobRef{ - Hash: blobHash, + err := bm.publishBlobs(ctx, &data.NewMessage{ + ResolvedData: data.Resolved{ + DataToPublish: []*fftypes.DataAndBlob{ + { + Data: &fftypes.Data{ + ID: dataID, + Blob: &fftypes.BlobRef{ + Hash: blobHash, + }, + }, + Blob: &fftypes.Blob{ + Hash: blobHash, + PayloadRef: "blob/1", + }, }, }, - Blob: &fftypes.Blob{ - Hash: blobHash, - PayloadRef: "blob/1", - }, }, }) assert.Regexp(t, "FF10240", err) diff --git a/internal/broadcast/message.go b/internal/broadcast/message.go index eb09557f0a..c9ba3fde24 100644 --- a/internal/broadcast/message.go +++ b/internal/broadcast/message.go @@ -19,10 +19,10 @@ package broadcast import ( "context" + "github.com/hyperledger/firefly/internal/data" "github.com/hyperledger/firefly/internal/i18n" "github.com/hyperledger/firefly/internal/log" "github.com/hyperledger/firefly/internal/sysmessaging" - "github.com/hyperledger/firefly/pkg/database" "github.com/hyperledger/firefly/pkg/fftypes" ) @@ -30,7 +30,10 @@ func (bm *broadcastManager) NewBroadcast(ns string, in *fftypes.MessageInOut) sy broadcast := &broadcastSender{ mgr: bm, namespace: ns, - msg: in, + msg: &data.NewMessage{ + Message: &in.Message, + InData: in.InlineData, + }, } broadcast.setDefaults() return broadcast @@ -52,8 +55,7 @@ func (bm *broadcastManager) BroadcastMessage(ctx context.Context, ns string, in type broadcastSender struct { mgr *broadcastManager namespace string - msg *fftypes.MessageInOut - data []*fftypes.Data + msg *data.NewMessage resolved bool } @@ -83,94 +85,77 @@ func (s *broadcastSender) SendAndWait(ctx context.Context) error { } func (s *broadcastSender) setDefaults() { - s.msg.Header.ID = fftypes.NewUUID() - s.msg.Header.Namespace = s.namespace - s.msg.State = fftypes.MessageStateReady - if s.msg.Header.Type == "" { - s.msg.Header.Type = fftypes.MessageTypeBroadcast + msg := s.msg.Message + msg.Header.ID = fftypes.NewUUID() + msg.Header.Namespace = s.namespace + msg.State = fftypes.MessageStateReady + if msg.Header.Type == "" { + msg.Header.Type = fftypes.MessageTypeBroadcast } // We only have one transaction type for broadcast currently - s.msg.Header.TxType = fftypes.TransactionTypeBatchPin + msg.Header.TxType = fftypes.TransactionTypeBatchPin } func (s *broadcastSender) resolveAndSend(ctx context.Context, method sendMethod) error { - sent := false - // We optimize the DB storage of all the parts of the message using transaction semantics (assuming those are supported by the DB plugin) - var dataToPublish []*fftypes.DataAndBlob - err := s.mgr.database.RunAsGroup(ctx, func(ctx context.Context) (err error) { - if !s.resolved { - if dataToPublish, err = s.resolve(ctx); err != nil { - return err - } - msgSizeEstimate := s.msg.EstimateSize(true) - if msgSizeEstimate > s.mgr.maxBatchPayloadLength { - return i18n.NewError(ctx, i18n.MsgTooLargeBroadcast, float64(msgSizeEstimate)/1024, float64(s.mgr.maxBatchPayloadLength)/1024) - } - s.resolved = true + if !s.resolved { + if err := s.resolve(ctx); err != nil { + return err } - - // For the simple case where we have no data to publish and aren't waiting for blockchain confirmation, - // insert the local message immediately within the same DB transaction. - // Otherwise, break out of the DB transaction (since those operations could take multiple seconds). - if len(dataToPublish) == 0 && method != methodSendAndWait { - sent = true - return s.sendInternal(ctx, method) + msgSizeEstimate := s.msg.Message.EstimateSize(true) + if msgSizeEstimate > s.mgr.maxBatchPayloadLength { + return i18n.NewError(ctx, i18n.MsgTooLargeBroadcast, float64(msgSizeEstimate)/1024, float64(s.mgr.maxBatchPayloadLength)/1024) } - return nil - }) - - if err != nil || sent { - return err - } - // Perform deferred processing - if len(dataToPublish) > 0 { - if err := s.mgr.publishBlobs(ctx, dataToPublish); err != nil { - return err + // Perform deferred processing + if len(s.msg.ResolvedData.DataToPublish) > 0 { + if err := s.mgr.publishBlobs(ctx, s.msg); err != nil { + return err + } } + s.resolved = true } return s.sendInternal(ctx, method) } -func (s *broadcastSender) resolve(ctx context.Context) ([]*fftypes.DataAndBlob, error) { +func (s *broadcastSender) resolve(ctx context.Context) error { + msg := s.msg.Message + // Resolve the sending identity - if s.msg.Header.Type != fftypes.MessageTypeDefinition || s.msg.Header.Tag != fftypes.SystemTagIdentityClaim { - if err := s.mgr.identity.ResolveInputSigningIdentity(ctx, s.msg.Header.Namespace, &s.msg.Header.SignerRef); err != nil { - return nil, i18n.WrapError(ctx, err, i18n.MsgAuthorInvalid) + if msg.Header.Type != fftypes.MessageTypeDefinition || msg.Header.Tag != fftypes.SystemTagIdentityClaim { + if err := s.mgr.identity.ResolveInputSigningIdentity(ctx, msg.Header.Namespace, &msg.Header.SignerRef); err != nil { + return i18n.WrapError(ctx, err, i18n.MsgAuthorInvalid) } } // The data manager is responsible for the heavy lifting of storing/validating all our in-line data elements - data, dataToPublish, err := s.mgr.data.ResolveInlineDataBroadcast(ctx, s.namespace, s.msg.InlineData) - s.data = data - s.msg.Message.Data = data.Refs() - return dataToPublish, err + err := s.mgr.data.ResolveInlineDataBroadcast(ctx, s.msg) + return err } func (s *broadcastSender) sendInternal(ctx context.Context, method sendMethod) (err error) { if method == methodSendAndWait { - out, err := s.mgr.syncasync.WaitForMessage(ctx, s.namespace, s.msg.Header.ID, s.Send) + out, err := s.mgr.syncasync.WaitForMessage(ctx, s.namespace, s.msg.Message.Header.ID, s.Send) if out != nil { - s.msg.Message = *out + *s.msg.Message = *out } return err } // Seal the message - if err := s.msg.Seal(ctx); err != nil { + msg := s.msg.Message + if err := msg.Seal(ctx); err != nil { return err } if method == methodPrepare { return nil } - // Store the message - this asynchronously triggers the next step in process - if err := s.mgr.database.UpsertMessage(ctx, &s.msg.Message, database.UpsertOptimizationNew); err != nil { + // Write the message + if err := s.mgr.data.WriteNewMessage(ctx, s.msg); err != nil { return err } - s.mgr.data.UpdateMessageCache(&s.msg.Message, s.data) - log.L(ctx).Infof("Sent broadcast message %s:%s sequence=%d datacount=%d", s.msg.Header.Namespace, s.msg.Header.ID, s.msg.Sequence, len(s.data)) + log.L(ctx).Infof("Sent broadcast message %s:%s sequence=%d datacount=%d", msg.Header.Namespace, msg.Header.ID, msg.Sequence, len(s.msg.ResolvedData.AllData)) return err } diff --git a/internal/broadcast/message_test.go b/internal/broadcast/message_test.go index bced577943..397fce10e4 100644 --- a/internal/broadcast/message_test.go +++ b/internal/broadcast/message_test.go @@ -24,14 +24,13 @@ import ( "io/ioutil" "testing" + "github.com/hyperledger/firefly/internal/data" "github.com/hyperledger/firefly/internal/syncasync" - "github.com/hyperledger/firefly/mocks/databasemocks" "github.com/hyperledger/firefly/mocks/dataexchangemocks" "github.com/hyperledger/firefly/mocks/datamocks" "github.com/hyperledger/firefly/mocks/identitymanagermocks" "github.com/hyperledger/firefly/mocks/sharedstoragemocks" "github.com/hyperledger/firefly/mocks/syncasyncmocks" - "github.com/hyperledger/firefly/pkg/database" "github.com/hyperledger/firefly/pkg/fftypes" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/mock" @@ -40,21 +39,12 @@ import ( func TestBroadcastMessageOk(t *testing.T) { bm, cancel := newTestBroadcastWithMetrics(t) defer cancel() - mdi := bm.database.(*databasemocks.Plugin) mdm := bm.data.(*datamocks.Manager) mim := bm.identity.(*identitymanagermocks.Manager) ctx := context.Background() - rag := mdi.On("RunAsGroup", ctx, mock.Anything) - rag.RunFn = func(a mock.Arguments) { - var fn = a[1].(func(context.Context) error) - rag.ReturnArguments = mock.Arguments{fn(a[0].(context.Context))} - } - mdm.On("ResolveInlineDataBroadcast", ctx, "ns1", mock.Anything).Return(fftypes.DataArray{ - {ID: fftypes.NewUUID(), Hash: fftypes.NewRandB32()}, - }, []*fftypes.DataAndBlob{}, nil) - mdm.On("UpdateMessageCache", mock.Anything, mock.Anything).Return() - mdi.On("UpsertMessage", ctx, mock.Anything, database.UpsertOptimizationNew).Return(nil) + mdm.On("ResolveInlineDataBroadcast", ctx, mock.Anything).Return(nil) + mdm.On("WriteNewMessage", mock.Anything, mock.Anything, mock.Anything).Return(nil) mim.On("ResolveInputSigningIdentity", ctx, "ns1", mock.Anything).Return(nil) msg, err := bm.BroadcastMessage(ctx, "ns1", &fftypes.MessageInOut{ @@ -71,32 +61,21 @@ func TestBroadcastMessageOk(t *testing.T) { }, }, false) assert.NoError(t, err) - assert.NotNil(t, msg.Data[0].ID) - assert.NotNil(t, msg.Data[0].Hash) assert.Equal(t, "ns1", msg.Header.Namespace) - mdi.AssertExpectations(t) + mim.AssertExpectations(t) mdm.AssertExpectations(t) } func TestBroadcastMessageWaitConfirmOk(t *testing.T) { bm, cancel := newTestBroadcast(t) defer cancel() - mdi := bm.database.(*databasemocks.Plugin) mdm := bm.data.(*datamocks.Manager) msa := bm.syncasync.(*syncasyncmocks.Bridge) mim := bm.identity.(*identitymanagermocks.Manager) ctx := context.Background() - rag := mdi.On("RunAsGroup", ctx, mock.Anything) - rag.RunFn = func(a mock.Arguments) { - var fn = a[1].(func(context.Context) error) - rag.ReturnArguments = mock.Arguments{fn(a[0].(context.Context))} - } - mdm.On("ResolveInlineDataBroadcast", ctx, "ns1", mock.Anything).Return(fftypes.DataArray{ - {ID: fftypes.NewUUID(), Hash: fftypes.NewRandB32()}, - }, []*fftypes.DataAndBlob{}, nil) - mdm.On("UpdateMessageCache", mock.Anything, mock.Anything).Return() + mdm.On("ResolveInlineDataBroadcast", ctx, mock.Anything).Return(nil) mim.On("ResolveInputSigningIdentity", ctx, "ns1", mock.Anything).Return(nil) replyMsg := &fftypes.Message{ @@ -111,7 +90,7 @@ func TestBroadcastMessageWaitConfirmOk(t *testing.T) { send(ctx) }). Return(replyMsg, nil) - mdi.On("UpsertMessage", ctx, mock.Anything, database.UpsertOptimizationNew).Return(nil) + mdm.On("WriteNewMessage", ctx, mock.Anything, mock.Anything).Return(nil) msg, err := bm.BroadcastMessage(ctx, "ns1", &fftypes.MessageInOut{ Message: fftypes.Message{ @@ -130,14 +109,13 @@ func TestBroadcastMessageWaitConfirmOk(t *testing.T) { assert.Equal(t, replyMsg, msg) assert.Equal(t, "ns1", msg.Header.Namespace) - mdi.AssertExpectations(t) + msa.AssertExpectations(t) mdm.AssertExpectations(t) } func TestBroadcastMessageWithBlobsOk(t *testing.T) { bm, cancel := newTestBroadcast(t) defer cancel() - mdi := bm.database.(*databasemocks.Plugin) mdm := bm.data.(*datamocks.Manager) mdx := bm.exchange.(*dataexchangemocks.Plugin) mps := bm.sharedstorage.(*sharedstoragemocks.Plugin) @@ -147,37 +125,40 @@ func TestBroadcastMessageWithBlobsOk(t *testing.T) { dataID := fftypes.NewUUID() ctx := context.Background() - rag := mdi.On("RunAsGroup", ctx, mock.Anything) - rag.RunFn = func(a mock.Arguments) { - var fn = a[1].(func(context.Context) error) - rag.ReturnArguments = mock.Arguments{fn(a[0].(context.Context))} - } - mdm.On("ResolveInlineDataBroadcast", ctx, "ns1", mock.Anything).Return(fftypes.DataArray{ - {ID: dataID, Hash: fftypes.NewRandB32()}, - }, []*fftypes.DataAndBlob{ - { - Data: &fftypes.Data{ - ID: dataID, - Blob: &fftypes.BlobRef{ - Hash: blobHash, + mdm.On("ResolveInlineDataBroadcast", ctx, mock.Anything). + Run(func(args mock.Arguments) { + newMsg := args[1].(*data.NewMessage) + newMsg.ResolvedData.AllData = fftypes.DataArray{ + {ID: dataID, Hash: fftypes.NewRandB32()}, + } + newMsg.ResolvedData.DataToPublish = []*fftypes.DataAndBlob{ + { + Data: &fftypes.Data{ + ID: dataID, + Blob: &fftypes.BlobRef{ + Hash: blobHash, + }, + }, + Blob: &fftypes.Blob{ + Hash: blobHash, + PayloadRef: "blob/1", + }, }, - }, - Blob: &fftypes.Blob{ - Hash: blobHash, - PayloadRef: "blob/1", - }, - }, - }, nil) - mdm.On("UpdateMessageCache", mock.Anything, mock.Anything).Return() + } + }). + Return(nil) mdx.On("DownloadBLOB", ctx, "blob/1").Return(ioutil.NopCloser(bytes.NewReader([]byte(`some data`))), nil) + var readStr string mps.On("PublishData", ctx, mock.MatchedBy(func(reader io.ReadCloser) bool { - b, err := ioutil.ReadAll(reader) - assert.NoError(t, err) - assert.Equal(t, "some data", string(b)) + if readStr == "" { // called again in AssertExpectationa + b, err := ioutil.ReadAll(reader) + assert.NoError(t, err) + readStr = string(b) + } + assert.Equal(t, "some data", readStr) return true - })).Return("payload-ref", nil) - mdi.On("UpdateData", ctx, mock.Anything, mock.Anything).Return(nil) - mdi.On("UpsertMessage", ctx, mock.Anything, database.UpsertOptimizationNew).Return(nil) + })).Return("payload-ref", nil).Once() + mdm.On("WriteNewMessage", ctx, mock.Anything, mock.Anything).Return(nil) mim.On("ResolveInputSigningIdentity", ctx, "ns1", mock.Anything).Return(nil) msg, err := bm.BroadcastMessage(ctx, "ns1", &fftypes.MessageInOut{ @@ -196,31 +177,30 @@ func TestBroadcastMessageWithBlobsOk(t *testing.T) { }, }, false) assert.NoError(t, err) - assert.NotNil(t, msg.Data[0].ID) - assert.NotNil(t, msg.Data[0].Hash) assert.Equal(t, "ns1", msg.Header.Namespace) - mdi.AssertExpectations(t) + mdx.AssertExpectations(t) + mps.AssertExpectations(t) mdm.AssertExpectations(t) + mim.AssertExpectations(t) } func TestBroadcastMessageTooLarge(t *testing.T) { bm, cancel := newTestBroadcast(t) bm.maxBatchPayloadLength = 1000000 defer cancel() - mdi := bm.database.(*databasemocks.Plugin) mdm := bm.data.(*datamocks.Manager) mim := bm.identity.(*identitymanagermocks.Manager) ctx := context.Background() - rag := mdi.On("RunAsGroup", ctx, mock.Anything) - rag.RunFn = func(a mock.Arguments) { - var fn = a[1].(func(context.Context) error) - rag.ReturnArguments = mock.Arguments{fn(a[0].(context.Context))} - } - mdm.On("ResolveInlineDataBroadcast", ctx, "ns1", mock.Anything).Return(fftypes.DataArray{ - {ID: fftypes.NewUUID(), Hash: fftypes.NewRandB32(), ValueSize: 1000001}, - }, []*fftypes.DataAndBlob{}, nil) + mdm.On("ResolveInlineDataBroadcast", ctx, mock.Anything).Run( + func(args mock.Arguments) { + newMsg := args[1].(*data.NewMessage) + newMsg.Message.Data = fftypes.DataRefs{ + {ID: fftypes.NewUUID(), Hash: fftypes.NewRandB32(), ValueSize: 1000001}, + } + }). + Return(nil) mim.On("ResolveInputSigningIdentity", ctx, "ns1", mock.Anything).Return(nil) _, err := bm.BroadcastMessage(ctx, "ns1", &fftypes.MessageInOut{ @@ -238,24 +218,17 @@ func TestBroadcastMessageTooLarge(t *testing.T) { }, true) assert.Regexp(t, "FF10327", err) - mdi.AssertExpectations(t) mdm.AssertExpectations(t) } func TestBroadcastMessageBadInput(t *testing.T) { bm, cancel := newTestBroadcast(t) defer cancel() - mdi := bm.database.(*databasemocks.Plugin) mdm := bm.data.(*datamocks.Manager) mim := bm.identity.(*identitymanagermocks.Manager) ctx := context.Background() - rag := mdi.On("RunAsGroup", ctx, mock.Anything) - rag.RunFn = func(a mock.Arguments) { - var fn = a[1].(func(context.Context) error) - rag.ReturnArguments = mock.Arguments{fn(a[0].(context.Context))} - } - mdm.On("ResolveInlineDataBroadcast", ctx, "ns1", mock.Anything).Return(nil, nil, fmt.Errorf("pop")) + mdm.On("ResolveInlineDataBroadcast", ctx, mock.Anything).Return(fmt.Errorf("pop")) mim.On("ResolveInputSigningIdentity", ctx, "ns1", mock.Anything).Return(nil) _, err := bm.BroadcastMessage(ctx, "ns1", &fftypes.MessageInOut{ @@ -265,7 +238,6 @@ func TestBroadcastMessageBadInput(t *testing.T) { }, false) assert.EqualError(t, err, "pop") - mdi.AssertExpectations(t) mdm.AssertExpectations(t) } @@ -290,7 +262,6 @@ func TestBroadcastMessageBadIdentity(t *testing.T) { func TestPublishBlobsSendMessageFail(t *testing.T) { bm, cancel := newTestBroadcast(t) defer cancel() - mdi := bm.database.(*databasemocks.Plugin) mdm := bm.data.(*datamocks.Manager) mdx := bm.exchange.(*dataexchangemocks.Plugin) mim := bm.identity.(*identitymanagermocks.Manager) @@ -299,28 +270,29 @@ func TestPublishBlobsSendMessageFail(t *testing.T) { dataID := fftypes.NewUUID() ctx := context.Background() - rag := mdi.On("RunAsGroup", ctx, mock.Anything) - rag.RunFn = func(a mock.Arguments) { - var fn = a[1].(func(context.Context) error) - rag.ReturnArguments = mock.Arguments{fn(a[0].(context.Context))} - } mim.On("ResolveInputSigningIdentity", ctx, "ns1", mock.Anything).Return(nil) - mdm.On("ResolveInlineDataBroadcast", ctx, "ns1", mock.Anything).Return(fftypes.DataArray{ - {ID: dataID, Hash: fftypes.NewRandB32()}, - }, []*fftypes.DataAndBlob{ - { - Data: &fftypes.Data{ - ID: dataID, - Blob: &fftypes.BlobRef{ - Hash: blobHash, + mdm.On("ResolveInlineDataBroadcast", ctx, mock.Anything). + Run(func(args mock.Arguments) { + newMsg := args[1].(*data.NewMessage) + newMsg.ResolvedData.AllData = fftypes.DataArray{ + {ID: dataID, Hash: fftypes.NewRandB32()}, + } + newMsg.ResolvedData.DataToPublish = []*fftypes.DataAndBlob{ + { + Data: &fftypes.Data{ + ID: dataID, + Blob: &fftypes.BlobRef{ + Hash: blobHash, + }, + }, + Blob: &fftypes.Blob{ + Hash: blobHash, + PayloadRef: "blob/1", + }, }, - }, - Blob: &fftypes.Blob{ - Hash: blobHash, - PayloadRef: "blob/1", - }, - }, - }, nil) + } + }). + Return(nil) mdx.On("DownloadBLOB", ctx, "blob/1").Return(nil, fmt.Errorf("pop")) _, err := bm.BroadcastMessage(ctx, "ns1", &fftypes.MessageInOut{ @@ -340,7 +312,6 @@ func TestPublishBlobsSendMessageFail(t *testing.T) { }, false) assert.Regexp(t, "FF10240", err) - mdi.AssertExpectations(t) mdm.AssertExpectations(t) mdx.AssertExpectations(t) mim.AssertExpectations(t) @@ -349,19 +320,11 @@ func TestPublishBlobsSendMessageFail(t *testing.T) { func TestBroadcastPrepare(t *testing.T) { bm, cancel := newTestBroadcast(t) defer cancel() - mdi := bm.database.(*databasemocks.Plugin) mdm := bm.data.(*datamocks.Manager) mim := bm.identity.(*identitymanagermocks.Manager) ctx := context.Background() - rag := mdi.On("RunAsGroup", ctx, mock.Anything) - rag.RunFn = func(a mock.Arguments) { - var fn = a[1].(func(context.Context) error) - rag.ReturnArguments = mock.Arguments{fn(a[0].(context.Context))} - } - mdm.On("ResolveInlineDataBroadcast", ctx, "ns1", mock.Anything).Return(fftypes.DataArray{ - {ID: fftypes.NewUUID(), Hash: fftypes.NewRandB32()}, - }, []*fftypes.DataAndBlob{}, nil) + mdm.On("ResolveInlineDataBroadcast", ctx, mock.Anything).Return(nil) mim.On("ResolveInputSigningIdentity", ctx, "ns1", mock.Anything).Return(nil) msg := &fftypes.MessageInOut{ @@ -381,10 +344,7 @@ func TestBroadcastPrepare(t *testing.T) { err := sender.Prepare(ctx) assert.NoError(t, err) - assert.NotNil(t, msg.Data[0].ID) - assert.NotNil(t, msg.Data[0].Hash) assert.Equal(t, "ns1", msg.Header.Namespace) - mdi.AssertExpectations(t) mdm.AssertExpectations(t) } diff --git a/internal/broadcast/namespace_test.go b/internal/broadcast/namespace_test.go index cd00abcd6d..f3956db85a 100644 --- a/internal/broadcast/namespace_test.go +++ b/internal/broadcast/namespace_test.go @@ -24,7 +24,6 @@ import ( "github.com/hyperledger/firefly/mocks/databasemocks" "github.com/hyperledger/firefly/mocks/datamocks" "github.com/hyperledger/firefly/mocks/identitymanagermocks" - "github.com/hyperledger/firefly/pkg/database" "github.com/hyperledger/firefly/pkg/fftypes" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/mock" @@ -69,10 +68,9 @@ func TestBroadcastNamespaceBroadcastOk(t *testing.T) { mim.On("ResolveInputSigningIdentity", mock.Anything, fftypes.SystemNamespace, mock.Anything).Return(nil) mdi.On("GetNamespace", mock.Anything, mock.Anything).Return(&fftypes.Namespace{Name: "ns1"}, nil) - mdi.On("UpsertData", mock.Anything, mock.Anything, database.UpsertOptimizationNew).Return(nil) mdm.On("CheckDatatype", mock.Anything, "ns1", mock.Anything).Return(nil) mdm.On("UpdateMessageCache", mock.Anything, mock.Anything).Return() - mdi.On("UpsertMessage", mock.Anything, mock.Anything, database.UpsertOptimizationNew).Return(nil) + mdm.On("WriteNewMessage", mock.Anything, mock.Anything).Return(nil) buff := strings.Builder{} buff.Grow(4097) for i := 0; i < 4097; i++ { diff --git a/internal/broadcast/tokenpool_test.go b/internal/broadcast/tokenpool_test.go index c5611b04ed..0cb4316951 100644 --- a/internal/broadcast/tokenpool_test.go +++ b/internal/broadcast/tokenpool_test.go @@ -24,7 +24,6 @@ import ( "github.com/hyperledger/firefly/mocks/databasemocks" "github.com/hyperledger/firefly/mocks/datamocks" "github.com/hyperledger/firefly/mocks/identitymanagermocks" - "github.com/hyperledger/firefly/pkg/database" "github.com/hyperledger/firefly/pkg/fftypes" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/mock" @@ -81,7 +80,6 @@ func TestBroadcastTokenPoolInvalid(t *testing.T) { func TestBroadcastTokenPoolBroadcastFail(t *testing.T) { bm, cancel := newTestBroadcast(t) defer cancel() - mdi := bm.database.(*databasemocks.Plugin) mdm := bm.data.(*datamocks.Manager) mim := bm.identity.(*identitymanagermocks.Manager) @@ -98,13 +96,11 @@ func TestBroadcastTokenPoolBroadcastFail(t *testing.T) { mim.On("ResolveInputSigningIdentity", mock.Anything, "ns1", mock.Anything).Return(nil) mdm.On("VerifyNamespaceExists", mock.Anything, "ns1").Return(nil) - mdi.On("UpsertData", mock.Anything, mock.Anything, database.UpsertOptimizationNew).Return(nil) - mdi.On("UpsertMessage", mock.Anything, mock.Anything, database.UpsertOptimizationNew).Return(fmt.Errorf("pop")) + mdm.On("WriteNewMessage", mock.Anything, mock.Anything).Return(fmt.Errorf("pop")) _, err := bm.BroadcastTokenPool(context.Background(), "ns1", pool, false) assert.EqualError(t, err, "pop") - mdi.AssertExpectations(t) mdm.AssertExpectations(t) mim.AssertExpectations(t) } @@ -112,7 +108,6 @@ func TestBroadcastTokenPoolBroadcastFail(t *testing.T) { func TestBroadcastTokenPoolOk(t *testing.T) { bm, cancel := newTestBroadcast(t) defer cancel() - mdi := bm.database.(*databasemocks.Plugin) mdm := bm.data.(*datamocks.Manager) mim := bm.identity.(*identitymanagermocks.Manager) @@ -129,14 +124,11 @@ func TestBroadcastTokenPoolOk(t *testing.T) { mim.On("ResolveInputSigningIdentity", mock.Anything, "ns1", mock.Anything).Return(nil) mdm.On("VerifyNamespaceExists", mock.Anything, "ns1").Return(nil) - mdm.On("UpdateMessageCache", mock.Anything, mock.Anything).Return() - mdi.On("UpsertData", mock.Anything, mock.Anything, database.UpsertOptimizationNew).Return(nil) - mdi.On("UpsertMessage", mock.Anything, mock.Anything, database.UpsertOptimizationNew).Return(nil) + mdm.On("WriteNewMessage", mock.Anything, mock.Anything).Return(nil) _, err := bm.BroadcastTokenPool(context.Background(), "ns1", pool, false) assert.NoError(t, err) - mdi.AssertExpectations(t) mdm.AssertExpectations(t) mim.AssertExpectations(t) } diff --git a/internal/data/data_manager.go b/internal/data/data_manager.go index 77fdfc7cb4..f72b34961e 100644 --- a/internal/data/data_manager.go +++ b/internal/data/data_manager.go @@ -473,6 +473,7 @@ func (dm *dataManager) resolveInlineData(ctx context.Context, newMessage *NewMes }) } } + newMessage.Message.Data = r.AllData.Refs() return nil } @@ -516,7 +517,12 @@ func (dm *dataManager) HydrateBatch(ctx context.Context, persistedBatch *fftypes } func (dm *dataManager) WriteNewMessage(ctx context.Context, newMsg *NewMessage) error { - return dm.messageWriter.WriteNewMessage(ctx, newMsg) + err := dm.messageWriter.WriteNewMessage(ctx, newMsg) + if err != nil { + return err + } + dm.UpdateMessageCache(newMsg.Message, newMsg.ResolvedData.AllData) + return nil } func (dm *dataManager) Close() { diff --git a/internal/data/data_manager_test.go b/internal/data/data_manager_test.go index 5a89a7207c..3330b550c1 100644 --- a/internal/data/data_manager_test.go +++ b/internal/data/data_manager_test.go @@ -395,6 +395,7 @@ func TestResolveInlineDataEmpty(t *testing.T) { err := dm.ResolveInlineDataPrivate(ctx, newMsg) assert.NoError(t, err) assert.Empty(t, newMsg.ResolvedData.AllData) + assert.Empty(t, newMsg.Message.Data) } @@ -414,6 +415,7 @@ func TestResolveInlineDataRefIDOnlyOK(t *testing.T) { err := dm.ResolveInlineDataPrivate(ctx, newMsg) assert.NoError(t, err) assert.Len(t, newMsg.ResolvedData.AllData, 1) + assert.Len(t, newMsg.Message.Data, 1) assert.Equal(t, dataID, newMsg.ResolvedData.AllData[0].ID) assert.Equal(t, dataHash, newMsg.ResolvedData.AllData[0].Hash) assert.Empty(t, newMsg.ResolvedData.NewData) @@ -444,6 +446,7 @@ func TestResolveInlineDataBroadcastDataToPublish(t *testing.T) { err := dm.ResolveInlineDataBroadcast(ctx, newMsg) assert.NoError(t, err) assert.Len(t, newMsg.ResolvedData.AllData, 1) + assert.Len(t, newMsg.Message.Data, 1) assert.Empty(t, newMsg.ResolvedData.NewData) assert.Len(t, newMsg.ResolvedData.DataToPublish, 1) assert.Equal(t, dataID, newMsg.ResolvedData.AllData[0].ID) @@ -538,6 +541,7 @@ func TestResolveInlineDataValueNoValidatorOK(t *testing.T) { assert.NoError(t, err) assert.Len(t, newMsg.ResolvedData.AllData, 1) assert.Len(t, newMsg.ResolvedData.NewData, 1) + assert.Len(t, newMsg.Message.Data, 1) assert.NotNil(t, newMsg.ResolvedData.AllData[0].ID) assert.NotNil(t, newMsg.ResolvedData.AllData[0].Hash) } diff --git a/mocks/datamocks/manager.go b/mocks/datamocks/manager.go index d149849925..e024c8ae64 100644 --- a/mocks/datamocks/manager.go +++ b/mocks/datamocks/manager.go @@ -198,59 +198,32 @@ func (_m *Manager) HydrateBatch(ctx context.Context, persistedBatch *fftypes.Bat return r0, r1 } -// ResolveInlineDataBroadcast provides a mock function with given fields: ctx, ns, inData -func (_m *Manager) ResolveInlineDataBroadcast(ctx context.Context, ns string, inData fftypes.InlineData) (fftypes.DataArray, []*fftypes.DataAndBlob, error) { - ret := _m.Called(ctx, ns, inData) - - var r0 fftypes.DataArray - if rf, ok := ret.Get(0).(func(context.Context, string, fftypes.InlineData) fftypes.DataArray); ok { - r0 = rf(ctx, ns, inData) - } else { - if ret.Get(0) != nil { - r0 = ret.Get(0).(fftypes.DataArray) - } - } +// ResolveInlineDataBroadcast provides a mock function with given fields: ctx, msg +func (_m *Manager) ResolveInlineDataBroadcast(ctx context.Context, msg *data.NewMessage) error { + ret := _m.Called(ctx, msg) - var r1 []*fftypes.DataAndBlob - if rf, ok := ret.Get(1).(func(context.Context, string, fftypes.InlineData) []*fftypes.DataAndBlob); ok { - r1 = rf(ctx, ns, inData) - } else { - if ret.Get(1) != nil { - r1 = ret.Get(1).([]*fftypes.DataAndBlob) - } - } - - var r2 error - if rf, ok := ret.Get(2).(func(context.Context, string, fftypes.InlineData) error); ok { - r2 = rf(ctx, ns, inData) + var r0 error + if rf, ok := ret.Get(0).(func(context.Context, *data.NewMessage) error); ok { + r0 = rf(ctx, msg) } else { - r2 = ret.Error(2) + r0 = ret.Error(0) } - return r0, r1, r2 + return r0 } -// ResolveInlineDataPrivate provides a mock function with given fields: ctx, ns, inData -func (_m *Manager) ResolveInlineDataPrivate(ctx context.Context, ns string, inData fftypes.InlineData) (fftypes.DataArray, error) { - ret := _m.Called(ctx, ns, inData) - - var r0 fftypes.DataArray - if rf, ok := ret.Get(0).(func(context.Context, string, fftypes.InlineData) fftypes.DataArray); ok { - r0 = rf(ctx, ns, inData) - } else { - if ret.Get(0) != nil { - r0 = ret.Get(0).(fftypes.DataArray) - } - } +// ResolveInlineDataPrivate provides a mock function with given fields: ctx, msg +func (_m *Manager) ResolveInlineDataPrivate(ctx context.Context, msg *data.NewMessage) error { + ret := _m.Called(ctx, msg) - var r1 error - if rf, ok := ret.Get(1).(func(context.Context, string, fftypes.InlineData) error); ok { - r1 = rf(ctx, ns, inData) + var r0 error + if rf, ok := ret.Get(0).(func(context.Context, *data.NewMessage) error); ok { + r0 = rf(ctx, msg) } else { - r1 = ret.Error(1) + r0 = ret.Error(0) } - return r0, r1 + return r0 } // UpdateMessageCache provides a mock function with given fields: msg, _a1 @@ -343,3 +316,17 @@ func (_m *Manager) VerifyNamespaceExists(ctx context.Context, ns string) error { return r0 } + +// WriteNewMessage provides a mock function with given fields: ctx, newMsg +func (_m *Manager) WriteNewMessage(ctx context.Context, newMsg *data.NewMessage) error { + ret := _m.Called(ctx, newMsg) + + var r0 error + if rf, ok := ret.Get(0).(func(context.Context, *data.NewMessage) error); ok { + r0 = rf(ctx, newMsg) + } else { + r0 = ret.Error(0) + } + + return r0 +} From dac3c81b2360b4068dc8744ae3673834d7a42ca9 Mon Sep 17 00:00:00 2001 From: Peter Broadhurst Date: Wed, 9 Mar 2022 22:45:15 -0500 Subject: [PATCH 03/11] Work through private messaging Signed-off-by: Peter Broadhurst --- internal/broadcast/definition.go | 20 ++-- internal/broadcast/manager_test.go | 28 +++--- internal/broadcast/message.go | 5 +- internal/data/data_manager.go | 14 ++- internal/data/data_manager_test.go | 60 +++++++---- internal/data/message_writer.go | 10 +- internal/data/message_writer_test.go | 14 ++- internal/orchestrator/orchestrator.go | 2 +- internal/orchestrator/orchestrator_test.go | 1 + internal/privatemessaging/message.go | 62 ++++++------ internal/privatemessaging/message_test.go | 110 +++++---------------- mocks/datamocks/manager.go | 10 +- 12 files changed, 159 insertions(+), 177 deletions(-) diff --git a/internal/broadcast/definition.go b/internal/broadcast/definition.go index 41e9ca1470..41b79705db 100644 --- a/internal/broadcast/definition.go +++ b/internal/broadcast/definition.go @@ -72,14 +72,16 @@ func (bm *broadcastManager) broadcastDefinitionCommon(ctx context.Context, ns st // Create a broadcast message referring to the data newMsg := &data.NewMessage{ - Message: &fftypes.Message{ - Header: fftypes.MessageHeader{ - Namespace: ns, - Type: fftypes.MessageTypeDefinition, - SignerRef: *signingIdentity, - Topics: fftypes.FFStringArray{def.Topic()}, - Tag: tag, - TxType: fftypes.TransactionTypeBatchPin, + Message: &fftypes.MessageInOut{ + Message: fftypes.Message{ + Header: fftypes.MessageHeader{ + Namespace: ns, + Type: fftypes.MessageTypeDefinition, + SignerRef: *signingIdentity, + Topics: fftypes.FFStringArray{def.Topic()}, + Tag: tag, + TxType: fftypes.TransactionTypeBatchPin, + }, }, }, ResolvedData: data.Resolved{ @@ -100,5 +102,5 @@ func (bm *broadcastManager) broadcastDefinitionCommon(ctx context.Context, ns st } else { err = sender.Send(ctx) } - return newMsg.Message, err + return &newMsg.Message.Message, err } diff --git a/internal/broadcast/manager_test.go b/internal/broadcast/manager_test.go index 00867306f7..818e4fb019 100644 --- a/internal/broadcast/manager_test.go +++ b/internal/broadcast/manager_test.go @@ -111,12 +111,14 @@ func TestBroadcastMessageGood(t *testing.T) { dataID := fftypes.NewUUID() dataHash := fftypes.NewRandB32() newMsg := &data.NewMessage{ - Message: &fftypes.Message{ - Header: fftypes.MessageHeader{ - ID: fftypes.NewUUID(), - }, - Data: fftypes.DataRefs{ - {ID: dataID, Hash: dataHash}, + Message: &fftypes.MessageInOut{ + Message: fftypes.Message{ + Header: fftypes.MessageHeader{ + ID: fftypes.NewUUID(), + }, + Data: fftypes.DataRefs{ + {ID: dataID, Hash: dataHash}, + }, }, }, ResolvedData: data.Resolved{ @@ -147,12 +149,14 @@ func TestBroadcastMessageBad(t *testing.T) { defer cancel() newMsg := &data.NewMessage{ - Message: &fftypes.Message{ - Header: fftypes.MessageHeader{ - ID: fftypes.NewUUID(), - }, - Data: fftypes.DataRefs{ - {ID: fftypes.NewUUID(), Hash: nil}, + Message: &fftypes.MessageInOut{ + Message: fftypes.Message{ + Header: fftypes.MessageHeader{ + ID: fftypes.NewUUID(), + }, + Data: fftypes.DataRefs{ + {ID: fftypes.NewUUID(), Hash: nil}, + }, }, }, } diff --git a/internal/broadcast/message.go b/internal/broadcast/message.go index c9ba3fde24..d12d4e4375 100644 --- a/internal/broadcast/message.go +++ b/internal/broadcast/message.go @@ -31,8 +31,7 @@ func (bm *broadcastManager) NewBroadcast(ns string, in *fftypes.MessageInOut) sy mgr: bm, namespace: ns, msg: &data.NewMessage{ - Message: &in.Message, - InData: in.InlineData, + Message: in, }, } broadcast.setDefaults() @@ -137,7 +136,7 @@ func (s *broadcastSender) sendInternal(ctx context.Context, method sendMethod) ( if method == methodSendAndWait { out, err := s.mgr.syncasync.WaitForMessage(ctx, s.namespace, s.msg.Message.Header.ID, s.Send) if out != nil { - *s.msg.Message = *out + s.msg.Message.Message = *out } return err } diff --git a/internal/data/data_manager.go b/internal/data/data_manager.go index f72b34961e..b2788d6d3f 100644 --- a/internal/data/data_manager.go +++ b/internal/data/data_manager.go @@ -49,7 +49,7 @@ type Manager interface { CopyBlobPStoDX(ctx context.Context, data *fftypes.Data) (blob *fftypes.Blob, err error) DownloadBLOB(ctx context.Context, ns, dataID string) (*fftypes.Blob, io.ReadCloser, error) HydrateBatch(ctx context.Context, persistedBatch *fftypes.BatchPersisted) (*fftypes.Batch, error) - Close() + WaitStop() } type dataManager struct { @@ -429,10 +429,14 @@ func (dm *dataManager) ResolveInlineDataBroadcast(ctx context.Context, newMessag func (dm *dataManager) resolveInlineData(ctx context.Context, newMessage *NewMessage, broadcast bool) (err error) { + if newMessage.Message == nil { + return i18n.NewError(ctx, i18n.MsgNilOrNullObject) + } + r := &newMessage.ResolvedData - inData := newMessage.InData + inData := newMessage.Message.InlineData msg := newMessage.Message - r.AllData = make(fftypes.DataArray, len(newMessage.InData)) + r.AllData = make(fftypes.DataArray, len(newMessage.Message.InlineData)) if broadcast { r.DataToPublish = make([]*fftypes.DataAndBlob, 0, len(inData)) } @@ -521,10 +525,10 @@ func (dm *dataManager) WriteNewMessage(ctx context.Context, newMsg *NewMessage) if err != nil { return err } - dm.UpdateMessageCache(newMsg.Message, newMsg.ResolvedData.AllData) + dm.UpdateMessageCache(&newMsg.Message.Message, newMsg.ResolvedData.AllData) return nil } -func (dm *dataManager) Close() { +func (dm *dataManager) WaitStop() { dm.messageWriter.close() } diff --git a/internal/data/data_manager_test.go b/internal/data/data_manager_test.go index 3330b550c1..6f6d3964db 100644 --- a/internal/data/data_manager_test.go +++ b/internal/data/data_manager_test.go @@ -44,7 +44,7 @@ func newTestDataManager(t *testing.T) (*dataManager, context.Context, func()) { assert.NoError(t, err) return dm.(*dataManager), ctx, func() { cancel() - dm.Close() + dm.WaitStop() } } @@ -52,14 +52,16 @@ func testNewMessage() (*fftypes.UUID, *fftypes.Bytes32, *NewMessage) { dataID := fftypes.NewUUID() dataHash := fftypes.NewRandB32() return dataID, dataHash, &NewMessage{ - Message: &fftypes.Message{ - Header: fftypes.MessageHeader{ - ID: fftypes.NewUUID(), - Namespace: "ns1", + Message: &fftypes.MessageInOut{ + Message: fftypes.Message{ + Header: fftypes.MessageHeader{ + ID: fftypes.NewUUID(), + Namespace: "ns1", + }, + }, + InlineData: fftypes.InlineData{ + {DataRef: fftypes.DataRef{ID: dataID, Hash: dataHash}}, }, - }, - InData: fftypes.InlineData{ - {DataRef: fftypes.DataRef{ID: dataID, Hash: dataHash}}, }, } } @@ -150,7 +152,7 @@ func TestWriteNewMessageE2E(t *testing.T) { mdi.On("GetDataByID", mock.Anything, data1.ID, true).Return(data1, nil).Once() _, _, newMsg1 := testNewMessage() - newMsg1.InData = fftypes.InlineData{ + newMsg1.Message.InlineData = fftypes.InlineData{ {DataRef: fftypes.DataRef{ ID: data1.ID, Hash: data1.Hash, @@ -159,7 +161,7 @@ func TestWriteNewMessageE2E(t *testing.T) { {Value: fftypes.JSONAnyPtr(`"message 1 - data C"`)}, } _, _, newMsg2 := testNewMessage() - newMsg2.InData = fftypes.InlineData{ + newMsg2.Message.InlineData = fftypes.InlineData{ {Value: fftypes.JSONAnyPtr(`"message 2 - data B"`)}, {Value: fftypes.JSONAnyPtr(`"message 2 - data C"`)}, } @@ -207,6 +209,7 @@ func TestWriteNewMessageE2E(t *testing.T) { mdi.AssertExpectations(t) } + func TestInitBadDeps(t *testing.T) { _, err := NewDataManager(context.Background(), nil, nil, nil) assert.Regexp(t, "FF10128", err) @@ -383,13 +386,15 @@ func TestResolveInlineDataEmpty(t *testing.T) { defer cancel() newMsg := &NewMessage{ - Message: &fftypes.Message{ - Header: fftypes.MessageHeader{ - ID: fftypes.NewUUID(), - Namespace: "ns1", + Message: &fftypes.MessageInOut{ + Message: fftypes.Message{ + Header: fftypes.MessageHeader{ + ID: fftypes.NewUUID(), + Namespace: "ns1", + }, }, + InlineData: fftypes.InlineData{}, }, - InData: fftypes.InlineData{}, } err := dm.ResolveInlineDataPrivate(ctx, newMsg) @@ -512,6 +517,14 @@ func TestResolveInlineDataRefBadHash(t *testing.T) { assert.Regexp(t, "FF10204", err) } +func TestResolveInlineDataNilMsg(t *testing.T) { + dm, ctx, cancel := newTestDataManager(t) + defer cancel() + + err := dm.ResolveInlineDataPrivate(ctx, &NewMessage{}) + assert.Regexp(t, "FF10368", err) +} + func TestResolveInlineDataRefLookkupFail(t *testing.T) { dm, ctx, cancel := newTestDataManager(t) defer cancel() @@ -533,7 +546,7 @@ func TestResolveInlineDataValueNoValidatorOK(t *testing.T) { mdi.On("UpsertData", ctx, mock.Anything, database.UpsertOptimizationNew).Return(nil) _, _, newMsg := testNewMessage() - newMsg.InData = fftypes.InlineData{ + newMsg.Message.InlineData = fftypes.InlineData{ {Value: fftypes.JSONAnyPtr(`{"some":"json"}`)}, } @@ -569,7 +582,7 @@ func TestResolveInlineDataValueWithValidation(t *testing.T) { }, nil) _, _, newMsg := testNewMessage() - newMsg.InData = fftypes.InlineData{ + newMsg.Message.InlineData = fftypes.InlineData{ { Datatype: &fftypes.DatatypeRef{ Name: "customer", @@ -586,7 +599,7 @@ func TestResolveInlineDataValueWithValidation(t *testing.T) { assert.NotNil(t, newMsg.ResolvedData.AllData[0].ID) assert.NotNil(t, newMsg.ResolvedData.AllData[0].Hash) - newMsg.InData = fftypes.InlineData{ + newMsg.Message.InlineData = fftypes.InlineData{ { Datatype: &fftypes.DatatypeRef{ Name: "customer", @@ -604,7 +617,7 @@ func TestResolveInlineDataNoRefOrValue(t *testing.T) { defer cancel() _, _, newMsg := testNewMessage() - newMsg.InData = fftypes.InlineData{ + newMsg.Message.InlineData = fftypes.InlineData{ { /* missing */ }, } @@ -1118,3 +1131,12 @@ func TestUpdateMessageCacheCRORequireBatchID(t *testing.T) { } } + +func TestWriteNewMessageFailNil(t *testing.T) { + + dm, ctx, cancel := newTestDataManager(t) + defer cancel() + + err := dm.WriteNewMessage(ctx, &NewMessage{}) + assert.Regexp(t, "FF10368", err) +} diff --git a/internal/data/message_writer.go b/internal/data/message_writer.go index e10862860f..16596ce261 100644 --- a/internal/data/message_writer.go +++ b/internal/data/message_writer.go @@ -27,8 +27,7 @@ import ( ) type NewMessage struct { - Message *fftypes.Message - InData fftypes.InlineData + Message *fftypes.MessageInOut ResolvedData Resolved } @@ -96,8 +95,11 @@ func newMessageWriter(ctx context.Context, di database.Plugin, conf *messageWrit func (mw *messageWriter) WriteNewMessage(ctx context.Context, newMsg *NewMessage) error { if mw.conf.workerCount > 0 { // Dispatch to background worker + if newMsg.Message == nil { + return i18n.NewError(ctx, i18n.MsgNilOrNullObject) + } nmi := &writeRequest{ - newMessage: newMsg.Message, + newMessage: &newMsg.Message.Message, newData: newMsg.ResolvedData.NewData, result: make(chan error), } @@ -109,7 +111,7 @@ func (mw *messageWriter) WriteNewMessage(ctx context.Context, newMsg *NewMessage return <-nmi.result } // Otherwise do it in-line on this context - return mw.writeMessages(ctx, []*fftypes.Message{newMsg.Message}, newMsg.ResolvedData.NewData) + return mw.writeMessages(ctx, []*fftypes.Message{&newMsg.Message.Message}, newMsg.ResolvedData.NewData) } // WriteData writes a piece of data independently of a message diff --git a/internal/data/message_writer_test.go b/internal/data/message_writer_test.go index e9c73df1cb..5c14c4392d 100644 --- a/internal/data/message_writer_test.go +++ b/internal/data/message_writer_test.go @@ -54,7 +54,9 @@ func TestNewMessageWriterNoConcurrency(t *testing.T) { func TestWriteNewMessageClosed(t *testing.T) { mw := newTestMessageWriter(t) mw.close() - err := mw.WriteNewMessage(mw.ctx, &NewMessage{}) + err := mw.WriteNewMessage(mw.ctx, &NewMessage{ + Message: &fftypes.MessageInOut{}, + }) assert.Regexp(t, "FF10158", err) } @@ -69,15 +71,17 @@ func TestWriteNewMessageSyncFallback(t *testing.T) { mw := newTestMessageWriterNoConcrrency(t) customCtx := context.WithValue(context.Background(), "dbtx", "on this context") - msg1 := &fftypes.Message{ - Header: fftypes.MessageHeader{ - ID: fftypes.NewUUID(), + msg1 := &fftypes.MessageInOut{ + Message: fftypes.Message{ + Header: fftypes.MessageHeader{ + ID: fftypes.NewUUID(), + }, }, } data1 := &fftypes.Data{ID: fftypes.NewUUID()} mdi := mw.database.(*databasemocks.Plugin) - mdi.On("InsertMessages", customCtx, []*fftypes.Message{msg1}).Return(nil) + mdi.On("InsertMessages", customCtx, []*fftypes.Message{&msg1.Message}).Return(nil) mdi.On("InsertDataArray", customCtx, fftypes.DataArray{data1}).Return(nil) err := mw.WriteNewMessage(customCtx, &NewMessage{ diff --git a/internal/orchestrator/orchestrator.go b/internal/orchestrator/orchestrator.go index 68fd59a841..edd34c11a1 100644 --- a/internal/orchestrator/orchestrator.go +++ b/internal/orchestrator/orchestrator.go @@ -250,7 +250,7 @@ func (or *orchestrator) WaitStop() { or.broadcast = nil } if or.data != nil { - or.data.Close() + or.data.WaitStop() or.data = nil } or.started = false diff --git a/internal/orchestrator/orchestrator_test.go b/internal/orchestrator/orchestrator_test.go index 7a7dd6950a..dc85133e12 100644 --- a/internal/orchestrator/orchestrator_test.go +++ b/internal/orchestrator/orchestrator_test.go @@ -592,6 +592,7 @@ func TestStartStopOk(t *testing.T) { or.mbm.On("WaitStop").Return(nil) or.mam.On("WaitStop").Return(nil) or.mti.On("WaitStop").Return(nil) + or.mdm.On("WaitStop").Return(nil) err := or.Start() assert.NoError(t, err) or.WaitStop() diff --git a/internal/privatemessaging/message.go b/internal/privatemessaging/message.go index e81458530d..b5f0891574 100644 --- a/internal/privatemessaging/message.go +++ b/internal/privatemessaging/message.go @@ -19,10 +19,10 @@ package privatemessaging import ( "context" + "github.com/hyperledger/firefly/internal/data" "github.com/hyperledger/firefly/internal/i18n" "github.com/hyperledger/firefly/internal/log" "github.com/hyperledger/firefly/internal/sysmessaging" - "github.com/hyperledger/firefly/pkg/database" "github.com/hyperledger/firefly/pkg/fftypes" ) @@ -30,7 +30,10 @@ func (pm *privateMessaging) NewMessage(ns string, in *fftypes.MessageInOut) sysm message := &messageSender{ mgr: pm, namespace: ns, - msg: in, + group: in.Group, + msg: &data.NewMessage{ + Message: in, + }, } message.setDefaults() return message @@ -65,8 +68,8 @@ func (pm *privateMessaging) RequestReply(ctx context.Context, ns string, in *fft type messageSender struct { mgr *privateMessaging namespace string - msg *fftypes.MessageInOut - data []*fftypes.Data + group *fftypes.InputGroup + msg *data.NewMessage resolved bool } @@ -94,19 +97,20 @@ func (s *messageSender) SendAndWait(ctx context.Context) error { } func (s *messageSender) setDefaults() { - s.msg.Header.ID = fftypes.NewUUID() - s.msg.Header.Namespace = s.namespace - s.msg.State = fftypes.MessageStateReady - if s.msg.Header.Type == "" { - s.msg.Header.Type = fftypes.MessageTypePrivate + msg := s.msg.Message + msg.Header.ID = fftypes.NewUUID() + msg.Header.Namespace = s.namespace + msg.State = fftypes.MessageStateReady + if msg.Header.Type == "" { + msg.Header.Type = fftypes.MessageTypePrivate } - switch s.msg.Header.TxType { + switch msg.Header.TxType { case fftypes.TransactionTypeUnpinned, fftypes.TransactionTypeNone: // "unpinned" used to be called "none" (before we introduced batching + a TX on unppinned sends) - s.msg.Header.TxType = fftypes.TransactionTypeUnpinned + msg.Header.TxType = fftypes.TransactionTypeUnpinned default: // the only other valid option is "batch_pin" - s.msg.Header.TxType = fftypes.TransactionTypeBatchPin + msg.Header.TxType = fftypes.TransactionTypeBatchPin } } @@ -116,10 +120,10 @@ func (s *messageSender) resolveAndSend(ctx context.Context, method sendMethod) e // We optimize the DB storage of all the parts of the message using transaction semantics (assuming those are supported by the DB plugin) err := s.mgr.database.RunAsGroup(ctx, func(ctx context.Context) (err error) { if !s.resolved { - if s.data, err = s.resolve(ctx); err != nil { + if err = s.resolve(ctx); err != nil { return err } - msgSizeEstimate := s.msg.EstimateSize(true) + msgSizeEstimate := s.msg.Message.EstimateSize(true) if msgSizeEstimate > s.mgr.maxBatchPayloadLength { return i18n.NewError(ctx, i18n.MsgTooLargePrivate, float64(msgSizeEstimate)/1024, float64(s.mgr.maxBatchPayloadLength)/1024) } @@ -141,37 +145,38 @@ func (s *messageSender) resolveAndSend(ctx context.Context, method sendMethod) e return s.sendInternal(ctx, method) } -func (s *messageSender) resolve(ctx context.Context) (fftypes.DataArray, error) { +func (s *messageSender) resolve(ctx context.Context) error { // Resolve the sending identity - if err := s.mgr.identity.ResolveInputSigningIdentity(ctx, s.msg.Header.Namespace, &s.msg.Header.SignerRef); err != nil { - return nil, i18n.WrapError(ctx, err, i18n.MsgAuthorInvalid) + msg := s.msg.Message + if err := s.mgr.identity.ResolveInputSigningIdentity(ctx, msg.Header.Namespace, &msg.Header.SignerRef); err != nil { + return i18n.WrapError(ctx, err, i18n.MsgAuthorInvalid) } // Resolve the member list into a group - if err := s.mgr.resolveRecipientList(ctx, s.msg); err != nil { - return nil, err + if err := s.mgr.resolveRecipientList(ctx, s.msg.Message); err != nil { + return err } // The data manager is responsible for the heavy lifting of storing/validating all our in-line data elements - data, err := s.mgr.data.ResolveInlineDataPrivate(ctx, s.namespace, s.msg.InlineData) - s.msg.Message.Data = data.Refs() - s.data = data - return data, err + err := s.mgr.data.ResolveInlineDataPrivate(ctx, s.msg) + return err } func (s *messageSender) sendInternal(ctx context.Context, method sendMethod) error { + msg := &s.msg.Message.Message + if method == methodSendAndWait { // Pass it to the sync-async handler to wait for the confirmation to come back in. // NOTE: Our caller makes sure we are not in a RunAsGroup (which would be bad) - out, err := s.mgr.syncasync.WaitForMessage(ctx, s.namespace, s.msg.Header.ID, s.Send) + out, err := s.mgr.syncasync.WaitForMessage(ctx, s.namespace, msg.Header.ID, s.Send) if out != nil { - s.msg.Message = *out + *msg = *out } return err } // Seal the message - if err := s.msg.Seal(ctx); err != nil { + if err := s.msg.Message.Seal(ctx); err != nil { return err } if method == methodPrepare { @@ -179,11 +184,10 @@ func (s *messageSender) sendInternal(ctx context.Context, method sendMethod) err } // Store the message - this asynchronously triggers the next step in process - if err := s.mgr.database.UpsertMessage(ctx, &s.msg.Message, database.UpsertOptimizationNew); err != nil { + if err := s.mgr.data.WriteNewMessage(ctx, s.msg); err != nil { return err } - s.mgr.data.UpdateMessageCache(&s.msg.Message, s.data) - log.L(ctx).Infof("Sent private message %s:%s sequence=%d", s.msg.Header.Namespace, s.msg.Header.ID, s.msg.Sequence) + log.L(ctx).Infof("Sent private message %s:%s sequence=%d", msg.Header.Namespace, msg.Header.ID, msg.Sequence) return nil } diff --git a/internal/privatemessaging/message_test.go b/internal/privatemessaging/message_test.go index 45231d9e22..6b11dbb6ec 100644 --- a/internal/privatemessaging/message_test.go +++ b/internal/privatemessaging/message_test.go @@ -22,6 +22,7 @@ import ( "testing" "github.com/hyperledger/firefly/internal/batch" + "github.com/hyperledger/firefly/internal/data" "github.com/hyperledger/firefly/internal/syncasync" "github.com/hyperledger/firefly/mocks/databasemocks" "github.com/hyperledger/firefly/mocks/dataexchangemocks" @@ -29,7 +30,6 @@ import ( "github.com/hyperledger/firefly/mocks/identitymanagermocks" "github.com/hyperledger/firefly/mocks/operationmocks" "github.com/hyperledger/firefly/mocks/syncasyncmocks" - "github.com/hyperledger/firefly/pkg/database" "github.com/hyperledger/firefly/pkg/fftypes" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/mock" @@ -84,12 +84,9 @@ func TestSendConfirmMessageE2EOk(t *testing.T) { mim.On("CachedIdentityLookup", pm.ctx, "org1").Return(intermediateOrg, false, nil) mim.On("CachedIdentityLookupByID", pm.ctx, rootOrg.ID).Return(rootOrg, nil) - dataID := fftypes.NewUUID() mdm := pm.data.(*datamocks.Manager) - mdm.On("ResolveInlineDataPrivate", pm.ctx, "ns1", mock.Anything).Return(fftypes.DataArray{ - {ID: dataID, Hash: fftypes.NewRandB32()}, - }, nil) - mdm.On("UpdateMessageCache", mock.Anything, mock.Anything).Return() + mdm.On("ResolveInlineDataPrivate", pm.ctx, mock.Anything).Return(nil) + mdm.On("WriteNewMessage", pm.ctx, mock.Anything).Return(nil).Once() mdi := pm.database.(*databasemocks.Plugin) mdi.On("GetIdentities", pm.ctx, mock.Anything).Return([]*fftypes.Identity{}, nil, nil).Once() @@ -110,7 +107,6 @@ func TestSendConfirmMessageE2EOk(t *testing.T) { send(pm.ctx) }). Return(retMsg, nil).Once() - mdi.On("UpsertMessage", pm.ctx, mock.Anything, database.UpsertOptimizationNew).Return(nil).Once() msg, err := pm.SendMessage(pm.ctx, "ns1", &fftypes.MessageInOut{ InlineData: fftypes.InlineData{ @@ -142,16 +138,12 @@ func TestSendUnpinnedMessageE2EOk(t *testing.T) { identity.Key = "localkey" }).Return(nil) - dataID := fftypes.NewUUID() groupID := fftypes.NewRandB32() mdm := pm.data.(*datamocks.Manager) - mdm.On("ResolveInlineDataPrivate", pm.ctx, "ns1", mock.Anything).Return(fftypes.DataArray{ - {ID: dataID, Hash: fftypes.NewRandB32()}, - }, nil) - mdm.On("UpdateMessageCache", mock.Anything, mock.Anything).Return() + mdm.On("ResolveInlineDataPrivate", pm.ctx, mock.Anything).Return(nil) + mdm.On("WriteNewMessage", pm.ctx, mock.Anything).Return(nil).Once() mdi := pm.database.(*databasemocks.Plugin) - mdi.On("UpsertMessage", pm.ctx, mock.Anything, database.UpsertOptimizationNew).Return(nil).Once() mdi.On("GetGroupByHash", pm.ctx, groupID).Return(&fftypes.Group{Hash: groupID}, nil) msg, err := pm.SendMessage(pm.ctx, "ns1", &fftypes.MessageInOut{ @@ -171,7 +163,6 @@ func TestSendUnpinnedMessageE2EOk(t *testing.T) { }, }, false) assert.NoError(t, err) - assert.Equal(t, *dataID, *msg.Data[0].ID) assert.NotNil(t, msg.Header.Group) mdm.AssertExpectations(t) @@ -224,54 +215,6 @@ func TestSendMessageBadIdentity(t *testing.T) { } -func TestSendMessageFail(t *testing.T) { - - pm, cancel := newTestPrivateMessaging(t) - defer cancel() - - mim := pm.identity.(*identitymanagermocks.Manager) - localOrg := newTestOrg("localorg") - localNode := newTestNode("node1", localOrg) - mim.On("ResolveInputSigningIdentity", pm.ctx, "ns1", mock.Anything).Return(nil) - mim.On("GetNodeOwnerOrg", pm.ctx).Return(localOrg, nil) - mim.On("ResolveInputSigningIdentity", pm.ctx, "ns1", mock.Anything).Run(func(args mock.Arguments) { - identity := args[2].(*fftypes.SignerRef) - identity.Author = "localorg" - identity.Key = "localkey" - }).Return(nil) - mim.On("CachedIdentityLookup", pm.ctx, "localorg").Return(localOrg, false, nil) - - mdi := pm.database.(*databasemocks.Plugin) - mdi.On("GetIdentities", pm.ctx, mock.Anything).Return([]*fftypes.Identity{localNode}, nil, nil) - mdi.On("GetGroups", pm.ctx, mock.Anything).Return([]*fftypes.Group{ - {Hash: fftypes.NewRandB32()}, - }, nil, nil) - mdi.On("UpsertMessage", pm.ctx, mock.Anything, database.UpsertOptimizationNew).Return(fmt.Errorf("pop")) - - dataID := fftypes.NewUUID() - mdm := pm.data.(*datamocks.Manager) - mdm.On("ResolveInlineDataPrivate", pm.ctx, "ns1", mock.Anything).Return(fftypes.DataArray{ - {ID: dataID, Hash: fftypes.NewRandB32()}, - }, nil) - - _, err := pm.SendMessage(pm.ctx, "ns1", &fftypes.MessageInOut{ - InlineData: fftypes.InlineData{ - {Value: fftypes.JSONAnyPtr(`{"some": "data"}`)}, - }, - Group: &fftypes.InputGroup{ - Members: []fftypes.MemberInput{ - {Identity: "localorg"}, - }, - }, - }, false) - assert.EqualError(t, err, "pop") - - mim.AssertExpectations(t) - mdi.AssertExpectations(t) - mdm.AssertExpectations(t) - -} - func TestResolveAndSendBadInlineData(t *testing.T) { pm, cancel := newTestPrivateMessaging(t) @@ -296,22 +239,24 @@ func TestResolveAndSendBadInlineData(t *testing.T) { }, nil, nil).Once() mdm := pm.data.(*datamocks.Manager) - mdm.On("ResolveInlineDataPrivate", pm.ctx, "ns1", mock.Anything).Return(nil, fmt.Errorf("pop")) + mdm.On("ResolveInlineDataPrivate", pm.ctx, mock.Anything).Return(fmt.Errorf("pop")) message := &messageSender{ mgr: pm, namespace: "ns1", - msg: &fftypes.MessageInOut{ - Message: fftypes.Message{Header: fftypes.MessageHeader{Namespace: "ns1"}}, - Group: &fftypes.InputGroup{ - Members: []fftypes.MemberInput{ - {Identity: "localorg"}, + msg: &data.NewMessage{ + Message: &fftypes.MessageInOut{ + Message: fftypes.Message{Header: fftypes.MessageHeader{Namespace: "ns1"}}, + Group: &fftypes.InputGroup{ + Members: []fftypes.MemberInput{ + {Identity: "localorg"}, + }, }, }, }, } - _, err := message.resolve(pm.ctx) + err := message.resolve(pm.ctx) assert.Regexp(t, "pop", err) mim.AssertExpectations(t) @@ -336,9 +281,12 @@ func TestSendUnpinnedMessageTooLarge(t *testing.T) { dataID := fftypes.NewUUID() groupID := fftypes.NewRandB32() mdm := pm.data.(*datamocks.Manager) - mdm.On("ResolveInlineDataPrivate", pm.ctx, "ns1", mock.Anything).Return(fftypes.DataArray{ - {ID: dataID, Hash: fftypes.NewRandB32(), ValueSize: 100001}, - }, nil) + mdm.On("ResolveInlineDataPrivate", pm.ctx, mock.Anything).Run(func(args mock.Arguments) { + newMsg := args[1].(*data.NewMessage) + newMsg.Message.Data = fftypes.DataRefs{ + {ID: dataID, Hash: fftypes.NewRandB32(), ValueSize: 100001}, + } + }).Return(nil) mdi := pm.database.(*databasemocks.Plugin) mdi.On("GetGroupByHash", pm.ctx, groupID).Return(&fftypes.Group{Hash: groupID}, nil) @@ -410,9 +358,7 @@ func TestMessagePrepare(t *testing.T) { }, nil, nil).Once() mdm := pm.data.(*datamocks.Manager) - mdm.On("ResolveInlineDataPrivate", pm.ctx, "ns1", mock.Anything).Return(fftypes.DataArray{ - {ID: fftypes.NewUUID(), Hash: fftypes.NewRandB32()}, - }, nil) + mdm.On("ResolveInlineDataPrivate", pm.ctx, mock.Anything).Return(nil) message := pm.NewMessage("ns1", &fftypes.MessageInOut{ Message: fftypes.Message{ @@ -483,15 +429,12 @@ func TestSendUnpinnedMessageInsertFail(t *testing.T) { return true })).Return(nil) - dataID := fftypes.NewUUID() groupID := fftypes.NewRandB32() mdm := pm.data.(*datamocks.Manager) - mdm.On("ResolveInlineDataPrivate", pm.ctx, "ns1", mock.Anything).Return(fftypes.DataArray{ - {ID: dataID, Hash: fftypes.NewRandB32()}, - }, nil) + mdm.On("ResolveInlineDataPrivate", pm.ctx, mock.Anything).Return(nil) + mdm.On("WriteNewMessage", pm.ctx, mock.Anything).Return(fmt.Errorf("pop")).Once() mdi := pm.database.(*databasemocks.Plugin) - mdi.On("UpsertMessage", pm.ctx, mock.Anything, database.UpsertOptimizationNew).Return(fmt.Errorf("pop")).Once() mdi.On("GetGroupByHash", pm.ctx, groupID).Return(&fftypes.Group{Hash: groupID}, nil) _, err := pm.SendMessage(pm.ctx, "ns1", &fftypes.MessageInOut{ @@ -669,15 +612,12 @@ func TestRequestReplySuccess(t *testing.T) { Return(nil, nil) mdm := pm.data.(*datamocks.Manager) - mdm.On("ResolveInlineDataPrivate", pm.ctx, "ns1", mock.Anything).Return(fftypes.DataArray{ - {ID: fftypes.NewUUID(), Hash: fftypes.NewRandB32()}, - }, nil) - mdm.On("UpdateMessageCache", mock.Anything, mock.Anything).Return() + mdm.On("ResolveInlineDataPrivate", pm.ctx, mock.Anything).Return(nil) + mdm.On("WriteNewMessage", pm.ctx, mock.Anything).Return(nil).Once() groupID := fftypes.NewRandB32() mdi := pm.database.(*databasemocks.Plugin) - mdi.On("UpsertMessage", pm.ctx, mock.Anything, database.UpsertOptimizationNew).Return(nil).Once() mdi.On("GetGroupByHash", pm.ctx, groupID).Return(&fftypes.Group{Hash: groupID}, nil) _, err := pm.RequestReply(pm.ctx, "ns1", &fftypes.MessageInOut{ diff --git a/mocks/datamocks/manager.go b/mocks/datamocks/manager.go index e024c8ae64..a42b7f5992 100644 --- a/mocks/datamocks/manager.go +++ b/mocks/datamocks/manager.go @@ -32,11 +32,6 @@ func (_m *Manager) CheckDatatype(ctx context.Context, ns string, datatype *fftyp return r0 } -// Close provides a mock function with given fields: -func (_m *Manager) Close() { - _m.Called() -} - // CopyBlobPStoDX provides a mock function with given fields: ctx, _a1 func (_m *Manager) CopyBlobPStoDX(ctx context.Context, _a1 *fftypes.Data) (*fftypes.Blob, error) { ret := _m.Called(ctx, _a1) @@ -317,6 +312,11 @@ func (_m *Manager) VerifyNamespaceExists(ctx context.Context, ns string) error { return r0 } +// WaitStop provides a mock function with given fields: +func (_m *Manager) WaitStop() { + _m.Called() +} + // WriteNewMessage provides a mock function with given fields: ctx, newMsg func (_m *Manager) WriteNewMessage(ctx context.Context, newMsg *data.NewMessage) error { ret := _m.Called(ctx, newMsg) From 8f4cee8f76031fb2f42b848b049c92f2a857128c Mon Sep 17 00:00:00 2001 From: Peter Broadhurst Date: Thu, 10 Mar 2022 00:12:53 -0500 Subject: [PATCH 04/11] Work through e2e Signed-off-by: Peter Broadhurst --- internal/broadcast/definition.go | 4 ++ internal/config/config.go | 2 +- internal/data/data_manager_test.go | 2 +- internal/data/message_writer.go | 12 +++--- internal/data/message_writer_test.go | 5 +++ internal/database/sqlcommon/message_sql.go | 32 ++++++++++++++-- .../database/sqlcommon/message_sql_test.go | 38 +++++++++++++++++-- 7 files changed, 82 insertions(+), 13 deletions(-) diff --git a/internal/broadcast/definition.go b/internal/broadcast/definition.go index 41b79705db..7778f99eed 100644 --- a/internal/broadcast/definition.go +++ b/internal/broadcast/definition.go @@ -82,10 +82,14 @@ func (bm *broadcastManager) broadcastDefinitionCommon(ctx context.Context, ns st Tag: tag, TxType: fftypes.TransactionTypeBatchPin, }, + Data: fftypes.DataRefs{ + {ID: d.ID, Hash: d.Hash, ValueSize: d.ValueSize}, + }, }, }, ResolvedData: data.Resolved{ NewData: fftypes.DataArray{d}, + AllData: fftypes.DataArray{d}, }, } diff --git a/internal/config/config.go b/internal/config/config.go index cda9d78338..5936a3e4d4 100644 --- a/internal/config/config.go +++ b/internal/config/config.go @@ -348,7 +348,7 @@ func Reset() { viper.SetDefault(string(MessageCacheTTL), "5m") viper.SetDefault(string(MessageWriterBatchMaxInserts), 200) viper.SetDefault(string(MessageWriterBatchTimeout), "100ms") - viper.SetDefault(string(MessageWriterCount), 1) + viper.SetDefault(string(MessageWriterCount), 3) viper.SetDefault(string(NamespacesDefault), "default") viper.SetDefault(string(NamespacesPredefined), fftypes.JSONObjectArray{{"name": "default", "description": "Default predefined namespace"}}) viper.SetDefault(string(OrchestratorStartupAttempts), 5) diff --git a/internal/data/data_manager_test.go b/internal/data/data_manager_test.go index 6f6d3964db..0c652c4276 100644 --- a/internal/data/data_manager_test.go +++ b/internal/data/data_manager_test.go @@ -33,6 +33,7 @@ import ( func newTestDataManager(t *testing.T) (*dataManager, context.Context, func()) { config.Reset() + config.Set(config.MessageWriterCount, 1) ctx, cancel := context.WithCancel(context.Background()) mdi := &databasemocks.Plugin{} mdi.On("Capabilities").Return(&database.Capabilities{ @@ -119,7 +120,6 @@ func TestValidateE2E(t *testing.T) { func TestWriteNewMessageE2E(t *testing.T) { - config.Reset() dm, ctx, cancel := newTestDataManager(t) defer cancel() diff --git a/internal/data/message_writer.go b/internal/data/message_writer.go index 16596ce261..98c92f4912 100644 --- a/internal/data/message_writer.go +++ b/internal/data/message_writer.go @@ -111,7 +111,9 @@ func (mw *messageWriter) WriteNewMessage(ctx context.Context, newMsg *NewMessage return <-nmi.result } // Otherwise do it in-line on this context - return mw.writeMessages(ctx, []*fftypes.Message{&newMsg.Message.Message}, newMsg.ResolvedData.NewData) + return mw.database.RunAsGroup(ctx, func(ctx context.Context) error { + return mw.writeMessages(ctx, []*fftypes.Message{&newMsg.Message.Message}, newMsg.ResolvedData.NewData) + }) } // WriteData writes a piece of data independently of a message @@ -185,13 +187,13 @@ func (mw *messageWriter) writerLoop(index int) { } func (mw *messageWriter) writeMessages(ctx context.Context, msgs []*fftypes.Message, data fftypes.DataArray) error { - if len(msgs) > 0 { - if err := mw.database.InsertMessages(ctx, msgs); err != nil { + if len(data) > 0 { + if err := mw.database.InsertDataArray(ctx, data); err != nil { return err } } - if len(data) > 0 { - if err := mw.database.InsertDataArray(ctx, data); err != nil { + if len(msgs) > 0 { + if err := mw.database.InsertMessages(ctx, msgs); err != nil { return err } } diff --git a/internal/data/message_writer_test.go b/internal/data/message_writer_test.go index 5c14c4392d..0f8bbea2e4 100644 --- a/internal/data/message_writer_test.go +++ b/internal/data/message_writer_test.go @@ -26,6 +26,7 @@ import ( "github.com/hyperledger/firefly/pkg/database" "github.com/hyperledger/firefly/pkg/fftypes" "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/mock" ) func newTestMessageWriter(t *testing.T) *messageWriter { @@ -81,6 +82,10 @@ func TestWriteNewMessageSyncFallback(t *testing.T) { data1 := &fftypes.Data{ID: fftypes.NewUUID()} mdi := mw.database.(*databasemocks.Plugin) + mdi.On("RunAsGroup", customCtx, mock.Anything).Run(func(args mock.Arguments) { + err := args[1].(func(context.Context) error)(customCtx) + assert.NoError(t, err) + }).Return(nil) mdi.On("InsertMessages", customCtx, []*fftypes.Message{&msg1.Message}).Return(nil) mdi.On("InsertDataArray", customCtx, fftypes.DataArray{data1}).Return(nil) diff --git a/internal/database/sqlcommon/message_sql.go b/internal/database/sqlcommon/message_sql.go index 87a6449a17..aa3d180348 100644 --- a/internal/database/sqlcommon/message_sql.go +++ b/internal/database/sqlcommon/message_sql.go @@ -194,12 +194,25 @@ func (s *SQLCommon) InsertMessages(ctx context.Context, messages []*fftypes.Mess defer s.rollbackTx(ctx, tx, autoCommit) if s.features.MultiRowInsert { - query := sq.Insert("messages").Columns(msgColumns...) + msgQuery := sq.Insert("messages").Columns(msgColumns...) + dataRefQuery := sq.Insert("messages_data").Columns( + "message_id", + "data_id", + "data_hash", + "data_idx", + ) + dataRefCount := 0 for _, message := range messages { - query = s.setMessageInsertValues(query, message) + msgQuery = s.setMessageInsertValues(msgQuery, message) + for idx, dataRef := range message.Data { + dataRefQuery = dataRefQuery.Values(message.Header.ID, dataRef.ID, dataRef.Hash, idx) + dataRefCount++ + } } sequences := make([]int64, len(messages)) - err := s.insertTxRows(ctx, tx, query, func() { + + // Use a single multi-row insert for the messages + err := s.insertTxRows(ctx, tx, msgQuery, func() { for i, message := range messages { message.Sequence = sequences[i] s.callbacks.OrderedUUIDCollectionNSEvent(database.CollectionMessages, fftypes.ChangeEventTypeCreated, message.Header.Namespace, message.Header.ID, message.Sequence) @@ -208,6 +221,15 @@ func (s *SQLCommon) InsertMessages(ctx context.Context, messages []*fftypes.Mess if err != nil { return err } + + // Use a single multi-row insert for the data refs + if dataRefCount > 0 { + dataRefSeqs := make([]int64, dataRefCount) + err = s.insertTxRows(ctx, tx, dataRefQuery, nil, dataRefSeqs, false) + if err != nil { + return err + } + } } else { // Fall back to individual inserts grouped in a TX for _, message := range messages { @@ -215,6 +237,10 @@ func (s *SQLCommon) InsertMessages(ctx context.Context, messages []*fftypes.Mess if err != nil { return err } + err = s.updateMessageDataRefs(ctx, tx, message, false) + if err != nil { + return err + } } } diff --git a/internal/database/sqlcommon/message_sql_test.go b/internal/database/sqlcommon/message_sql_test.go index 9145268806..133f3f67e0 100644 --- a/internal/database/sqlcommon/message_sql_test.go +++ b/internal/database/sqlcommon/message_sql_test.go @@ -290,16 +290,20 @@ func TestInsertMessagesMultiRowOK(t *testing.T) { s.features.MultiRowInsert = true s.fakePSQLInsert = true - msg1 := &fftypes.Message{Header: fftypes.MessageHeader{ID: fftypes.NewUUID(), Namespace: "ns1"}} - msg2 := &fftypes.Message{Header: fftypes.MessageHeader{ID: fftypes.NewUUID(), Namespace: "ns1"}} + msg1 := &fftypes.Message{Header: fftypes.MessageHeader{ID: fftypes.NewUUID(), Namespace: "ns1"}, Data: fftypes.DataRefs{{ID: fftypes.NewUUID()}}} + msg2 := &fftypes.Message{Header: fftypes.MessageHeader{ID: fftypes.NewUUID(), Namespace: "ns1"}, Data: fftypes.DataRefs{{ID: fftypes.NewUUID()}}} s.callbacks.On("OrderedUUIDCollectionNSEvent", database.CollectionMessages, fftypes.ChangeEventTypeCreated, "ns1", msg1.Header.ID, int64(1001)) s.callbacks.On("OrderedUUIDCollectionNSEvent", database.CollectionMessages, fftypes.ChangeEventTypeCreated, "ns1", msg2.Header.ID, int64(1002)) mock.ExpectBegin() - mock.ExpectQuery("INSERT.*").WillReturnRows(sqlmock.NewRows([]string{sequenceColumn}). + mock.ExpectQuery("INSERT.*messages").WillReturnRows(sqlmock.NewRows([]string{sequenceColumn}). AddRow(int64(1001)). AddRow(int64(1002)), ) + mock.ExpectQuery("INSERT.*messages_data").WillReturnRows(sqlmock.NewRows([]string{sequenceColumn}). + AddRow(int64(1003)). + AddRow(int64(1004)), + ) mock.ExpectCommit() err := s.InsertMessages(context.Background(), []*fftypes.Message{msg1, msg2}) assert.NoError(t, err) @@ -307,6 +311,22 @@ func TestInsertMessagesMultiRowOK(t *testing.T) { s.callbacks.AssertExpectations(t) } +func TestInsertMessagesMultiRowDataRefsFail(t *testing.T) { + s, mock := newMockProvider().init() + s.features.MultiRowInsert = true + s.fakePSQLInsert = true + + msg1 := &fftypes.Message{Header: fftypes.MessageHeader{ID: fftypes.NewUUID(), Namespace: "ns1"}, Data: fftypes.DataRefs{{ID: fftypes.NewUUID()}}} + + mock.ExpectBegin() + mock.ExpectQuery("INSERT.*messages").WillReturnRows(sqlmock.NewRows([]string{sequenceColumn}).AddRow(int64(1001))) + mock.ExpectQuery("INSERT.*messages_data").WillReturnError(fmt.Errorf("pop")) + err := s.InsertMessages(context.Background(), []*fftypes.Message{msg1}) + assert.Regexp(t, "FF10116", err) + assert.NoError(t, mock.ExpectationsWereMet()) + s.callbacks.AssertExpectations(t) +} + func TestInsertMessagesMultiRowFail(t *testing.T) { s, mock := newMockProvider().init() s.features.MultiRowInsert = true @@ -331,6 +351,18 @@ func TestInsertMessagesSingleRowFail(t *testing.T) { s.callbacks.AssertExpectations(t) } +func TestInsertMessagesSingleRowFailDataRefs(t *testing.T) { + s, mock := newMockProvider().init() + msg1 := &fftypes.Message{Header: fftypes.MessageHeader{ID: fftypes.NewUUID(), Namespace: "ns1"}, Data: fftypes.DataRefs{{ID: fftypes.NewUUID(), Hash: fftypes.NewRandB32()}}} + mock.ExpectBegin() + mock.ExpectExec("INSERT.*messages").WillReturnResult(sqlmock.NewResult(1, 1)) + mock.ExpectExec("INSERT.*messages_data").WillReturnError(fmt.Errorf("pop")) + err := s.InsertMessages(context.Background(), []*fftypes.Message{msg1}) + assert.Regexp(t, "FF10116", err) + assert.NoError(t, mock.ExpectationsWereMet()) + s.callbacks.AssertExpectations(t) +} + func TestReplaceMessageFailBegin(t *testing.T) { s, mock := newMockProvider().init() mock.ExpectBegin().WillReturnError(fmt.Errorf("pop")) From b4072837ef356f5af68ba485d6d9597a75281cb8 Mon Sep 17 00:00:00 2001 From: Peter Broadhurst Date: Thu, 10 Mar 2022 08:27:17 -0500 Subject: [PATCH 05/11] Fix token transfer message send to write data Signed-off-by: Peter Broadhurst --- internal/assets/token_transfer.go | 9 +++++---- internal/assets/token_transfer_test.go | 12 +++--------- test/e2e/e2e_test.go | 6 ++++-- 3 files changed, 12 insertions(+), 15 deletions(-) diff --git a/internal/assets/token_transfer.go b/internal/assets/token_transfer.go index 2e26fbfa83..b64e80a602 100644 --- a/internal/assets/token_transfer.go +++ b/internal/assets/token_transfer.go @@ -54,6 +54,7 @@ type transferSender struct { namespace string transfer *fftypes.TokenTransferInput resolved bool + msgSender sysmessaging.MessageSender } // sendMethod is the specific operation requested of the transferSender. @@ -189,14 +190,14 @@ func (s *transferSender) resolveAndSend(ctx context.Context, method sendMethod) return s.sendInternal(ctx, method) } -func (s *transferSender) resolve(ctx context.Context) error { +func (s *transferSender) resolve(ctx context.Context) (err error) { // Resolve the attached message if s.transfer.Message != nil { - sender, err := s.buildTransferMessage(ctx, s.namespace, s.transfer.Message) + s.msgSender, err = s.buildTransferMessage(ctx, s.namespace, s.transfer.Message) if err != nil { return err } - if err = sender.Prepare(ctx); err != nil { + if err = s.msgSender.Prepare(ctx); err != nil { return err } s.transfer.TokenTransfer.Message = s.transfer.Message.Header.ID @@ -257,7 +258,7 @@ func (s *transferSender) sendInternal(ctx context.Context, method sendMethod) er if s.transfer.Message != nil { s.transfer.Message.State = fftypes.MessageStateStaged - err = s.mgr.database.UpsertMessage(ctx, &s.transfer.Message.Message, database.UpsertOptimizationNew) + err = s.msgSender.Send(ctx) } return err }) diff --git a/internal/assets/token_transfer_test.go b/internal/assets/token_transfer_test.go index 16b3653d44..6a03eed39c 100644 --- a/internal/assets/token_transfer_test.go +++ b/internal/assets/token_transfer_test.go @@ -800,9 +800,7 @@ func TestTransferTokensWithBroadcastMessage(t *testing.T) { mdi.On("InsertOperation", context.Background(), mock.Anything).Return(nil) mbm.On("NewBroadcast", "ns1", transfer.Message).Return(mms) mms.On("Prepare", context.Background()).Return(nil) - mdi.On("UpsertMessage", context.Background(), mock.MatchedBy(func(msg *fftypes.Message) bool { - return msg.State == fftypes.MessageStateStaged - }), database.UpsertOptimizationNew).Return(nil) + mms.On("Send", context.Background()).Return(nil) mom.On("RunOperation", context.Background(), mock.MatchedBy(func(op *fftypes.PreparedOperation) bool { data := op.Data.(transferData) return op.Type == fftypes.OpTypeTokenTransfer && data.Pool == pool && data.Transfer == &transfer.TokenTransfer @@ -900,9 +898,7 @@ func TestTransferTokensWithPrivateMessage(t *testing.T) { mdi.On("InsertOperation", context.Background(), mock.Anything).Return(nil) mpm.On("NewMessage", "ns1", transfer.Message).Return(mms) mms.On("Prepare", context.Background()).Return(nil) - mdi.On("UpsertMessage", context.Background(), mock.MatchedBy(func(msg *fftypes.Message) bool { - return msg.State == fftypes.MessageStateStaged - }), database.UpsertOptimizationNew).Return(nil) + mms.On("Send", context.Background()).Return(nil) mom.On("RunOperation", context.Background(), mock.MatchedBy(func(op *fftypes.PreparedOperation) bool { data := op.Data.(transferData) return op.Type == fftypes.OpTypeTokenTransfer && data.Pool == pool && data.Transfer == &transfer.TokenTransfer @@ -1047,9 +1043,7 @@ func TestTransferTokensWithBroadcastConfirm(t *testing.T) { mth.On("SubmitNewTransaction", context.Background(), "ns1", fftypes.TransactionTypeTokenTransfer).Return(fftypes.NewUUID(), nil) mbm.On("NewBroadcast", "ns1", transfer.Message).Return(mms) mms.On("Prepare", context.Background()).Return(nil) - mdi.On("UpsertMessage", context.Background(), mock.MatchedBy(func(msg *fftypes.Message) bool { - return msg.State == fftypes.MessageStateStaged - }), database.UpsertOptimizationNew).Return(nil) + mms.On("Send", context.Background()).Return(nil) msa.On("WaitForMessage", context.Background(), "ns1", mock.Anything, mock.Anything). Run(func(args mock.Arguments) { send := args[3].(syncasync.RequestSender) diff --git a/test/e2e/e2e_test.go b/test/e2e/e2e_test.go index 730beed3da..56d7e398f1 100644 --- a/test/e2e/e2e_test.go +++ b/test/e2e/e2e_test.go @@ -336,10 +336,12 @@ func wsReader(conn *websocket.Conn, dbChanges bool) (chan *fftypes.EventDelivery func waitForEvent(t *testing.T, c chan *fftypes.EventDelivery, eventType fftypes.EventType, ref *fftypes.UUID) { for { - eventDelivery := <-c - if eventDelivery.Type == eventType && (ref == nil || *ref == *eventDelivery.Reference) { + ed := <-c + if ed.Type == eventType && (ref == nil || *ref == *ed.Reference) { + t.Logf("Detected '%s' event for ref '%s'", ed.Type, ed.Reference) return } + t.Logf("Ignored event '%s'", ed.ID) } } From a7a0071b8ce088ee677b6e9244507cca9e47ce60 Mon Sep 17 00:00:00 2001 From: Peter Broadhurst Date: Thu, 10 Mar 2022 08:31:19 -0500 Subject: [PATCH 06/11] Tested with e2e in psql too Signed-off-by: Peter Broadhurst --- internal/config/config.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/internal/config/config.go b/internal/config/config.go index 5936a3e4d4..4a305e7163 100644 --- a/internal/config/config.go +++ b/internal/config/config.go @@ -347,7 +347,7 @@ func Reset() { viper.SetDefault(string(MessageCacheSize), "50Mb") viper.SetDefault(string(MessageCacheTTL), "5m") viper.SetDefault(string(MessageWriterBatchMaxInserts), 200) - viper.SetDefault(string(MessageWriterBatchTimeout), "100ms") + viper.SetDefault(string(MessageWriterBatchTimeout), "50ms") viper.SetDefault(string(MessageWriterCount), 3) viper.SetDefault(string(NamespacesDefault), "default") viper.SetDefault(string(NamespacesPredefined), fftypes.JSONObjectArray{{"name": "default", "description": "Default predefined namespace"}}) From 889f133dcc05adad6c1373d8df1eb54b7be0afc7 Mon Sep 17 00:00:00 2001 From: Peter Broadhurst Date: Thu, 10 Mar 2022 10:33:16 -0500 Subject: [PATCH 07/11] Avoid any DB TX while executing sends Signed-off-by: Peter Broadhurst --- internal/assets/token_transfer.go | 12 ++-- internal/assets/token_transfer_test.go | 58 +++++++++++++++++++ internal/data/data_manager.go | 4 ++ internal/privatemessaging/message.go | 31 +++------- .../privatemessaging/privatemessaging_test.go | 7 --- 5 files changed, 78 insertions(+), 34 deletions(-) diff --git a/internal/assets/token_transfer.go b/internal/assets/token_transfer.go index b64e80a602..31977e50dc 100644 --- a/internal/assets/token_transfer.go +++ b/internal/assets/token_transfer.go @@ -256,16 +256,20 @@ func (s *transferSender) sendInternal(ctx context.Context, method sendMethod) er return err } - if s.transfer.Message != nil { - s.transfer.Message.State = fftypes.MessageStateStaged - err = s.msgSender.Send(ctx) - } return err }) if err != nil { return err } + // Write the transfer message outside of any DB transaction, as it will use the background message writer. + if s.transfer.Message != nil { + s.transfer.Message.State = fftypes.MessageStateStaged + if err = s.msgSender.Send(ctx); err != nil { + return err + } + } + return s.mgr.operations.RunOperation(ctx, opTransfer(op, pool, &s.transfer.TokenTransfer)) } diff --git a/internal/assets/token_transfer_test.go b/internal/assets/token_transfer_test.go index 6a03eed39c..87949b0047 100644 --- a/internal/assets/token_transfer_test.go +++ b/internal/assets/token_transfer_test.go @@ -819,6 +819,64 @@ func TestTransferTokensWithBroadcastMessage(t *testing.T) { mom.AssertExpectations(t) } +func TestTransferTokensWithBroadcastMessageSendFail(t *testing.T) { + am, cancel := newTestAssets(t) + defer cancel() + + msgID := fftypes.NewUUID() + hash := fftypes.NewRandB32() + transfer := &fftypes.TokenTransferInput{ + TokenTransfer: fftypes.TokenTransfer{ + From: "A", + To: "B", + Amount: *fftypes.NewFFBigInt(5), + }, + Pool: "pool1", + Message: &fftypes.MessageInOut{ + Message: fftypes.Message{ + Header: fftypes.MessageHeader{ + ID: msgID, + }, + Hash: hash, + }, + InlineData: fftypes.InlineData{ + { + Value: fftypes.JSONAnyPtr("test data"), + }, + }, + }, + } + pool := &fftypes.TokenPool{ + State: fftypes.TokenPoolStateConfirmed, + } + + mdi := am.database.(*databasemocks.Plugin) + mim := am.identity.(*identitymanagermocks.Manager) + mbm := am.broadcast.(*broadcastmocks.Manager) + mms := &sysmessagingmocks.MessageSender{} + mth := am.txHelper.(*txcommonmocks.Helper) + mom := am.operations.(*operationmocks.Manager) + mim.On("NormalizeSigningKey", context.Background(), "", identity.KeyNormalizationBlockchainPlugin).Return("0x12345", nil) + mdi.On("GetTokenPool", context.Background(), "ns1", "pool1").Return(pool, nil) + mth.On("SubmitNewTransaction", context.Background(), "ns1", fftypes.TransactionTypeTokenTransfer).Return(fftypes.NewUUID(), nil) + mdi.On("InsertOperation", context.Background(), mock.Anything).Return(nil) + mbm.On("NewBroadcast", "ns1", transfer.Message).Return(mms) + mms.On("Prepare", context.Background()).Return(nil) + mms.On("Send", context.Background()).Return(fmt.Errorf("pop")) + + _, err := am.TransferTokens(context.Background(), "ns1", transfer, false) + assert.Regexp(t, "pop", err) + assert.Equal(t, *msgID, *transfer.TokenTransfer.Message) + assert.Equal(t, *hash, *transfer.TokenTransfer.MessageHash) + + mbm.AssertExpectations(t) + mim.AssertExpectations(t) + mdi.AssertExpectations(t) + mms.AssertExpectations(t) + mth.AssertExpectations(t) + mom.AssertExpectations(t) +} + func TestTransferTokensWithBroadcastPrepareFail(t *testing.T) { am, cancel := newTestAssets(t) defer cancel() diff --git a/internal/data/data_manager.go b/internal/data/data_manager.go index b2788d6d3f..12b6ff2850 100644 --- a/internal/data/data_manager.go +++ b/internal/data/data_manager.go @@ -520,6 +520,10 @@ func (dm *dataManager) HydrateBatch(ctx context.Context, persistedBatch *fftypes return batch, nil } +// WriteNewMessage dispatches the writing of the message and assocated data, then blocks until the background +// worker (or foreground if no DB concurrency) has written. The caller MUST NOT call this inside of a +// DB RunAsGroup - because if a large number of routines enter the same function they could starve the background +// worker of the spare connection required to execute (and thus deadlock). func (dm *dataManager) WriteNewMessage(ctx context.Context, newMsg *NewMessage) error { err := dm.messageWriter.WriteNewMessage(ctx, newMsg) if err != nil { diff --git a/internal/privatemessaging/message.go b/internal/privatemessaging/message.go index b5f0891574..39513ea3a5 100644 --- a/internal/privatemessaging/message.go +++ b/internal/privatemessaging/message.go @@ -115,31 +115,16 @@ func (s *messageSender) setDefaults() { } func (s *messageSender) resolveAndSend(ctx context.Context, method sendMethod) error { - sent := false - - // We optimize the DB storage of all the parts of the message using transaction semantics (assuming those are supported by the DB plugin) - err := s.mgr.database.RunAsGroup(ctx, func(ctx context.Context) (err error) { - if !s.resolved { - if err = s.resolve(ctx); err != nil { - return err - } - msgSizeEstimate := s.msg.Message.EstimateSize(true) - if msgSizeEstimate > s.mgr.maxBatchPayloadLength { - return i18n.NewError(ctx, i18n.MsgTooLargePrivate, float64(msgSizeEstimate)/1024, float64(s.mgr.maxBatchPayloadLength)/1024) - } - s.resolved = true - } - // If we aren't waiting for blockchain confirmation, insert the local message immediately within the same DB transaction. - if method != methodSendAndWait { - err = s.sendInternal(ctx, method) - sent = true + if !s.resolved { + if err := s.resolve(ctx); err != nil { + return err } - return err - }) - - if err != nil || sent { - return err + msgSizeEstimate := s.msg.Message.EstimateSize(true) + if msgSizeEstimate > s.mgr.maxBatchPayloadLength { + return i18n.NewError(ctx, i18n.MsgTooLargePrivate, float64(msgSizeEstimate)/1024, float64(s.mgr.maxBatchPayloadLength)/1024) + } + s.resolved = true } return s.sendInternal(ctx, method) diff --git a/internal/privatemessaging/privatemessaging_test.go b/internal/privatemessaging/privatemessaging_test.go index 04c2b2809e..e6a2f4a19b 100644 --- a/internal/privatemessaging/privatemessaging_test.go +++ b/internal/privatemessaging/privatemessaging_test.go @@ -73,13 +73,6 @@ func newTestPrivateMessagingCommon(t *testing.T, metricsEnabled bool) (*privateM mmi.On("IsMetricsEnabled").Return(metricsEnabled) mom.On("RegisterHandler", mock.Anything, mock.Anything, mock.Anything) - rag := mdi.On("RunAsGroup", mock.Anything, mock.Anything).Maybe() - rag.RunFn = func(a mock.Arguments) { - rag.ReturnArguments = mock.Arguments{ - a[1].(func(context.Context) error)(a[0].(context.Context)), - } - } - ctx, cancel := context.WithCancel(context.Background()) pm, err := NewPrivateMessaging(ctx, mdi, mim, mdx, mbi, mba, mdm, msa, mbp, mmi, mom) assert.NoError(t, err) From a7aa64b172286a6fb5159ead8613b8aa6dfbd054 Mon Sep 17 00:00:00 2001 From: Peter Broadhurst Date: Thu, 10 Mar 2022 10:48:02 -0500 Subject: [PATCH 08/11] Tuning Signed-off-by: Peter Broadhurst --- internal/config/config.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/internal/config/config.go b/internal/config/config.go index 4a305e7163..a36edabbcc 100644 --- a/internal/config/config.go +++ b/internal/config/config.go @@ -347,8 +347,8 @@ func Reset() { viper.SetDefault(string(MessageCacheSize), "50Mb") viper.SetDefault(string(MessageCacheTTL), "5m") viper.SetDefault(string(MessageWriterBatchMaxInserts), 200) - viper.SetDefault(string(MessageWriterBatchTimeout), "50ms") - viper.SetDefault(string(MessageWriterCount), 3) + viper.SetDefault(string(MessageWriterBatchTimeout), "200ms") + viper.SetDefault(string(MessageWriterCount), 1) viper.SetDefault(string(NamespacesDefault), "default") viper.SetDefault(string(NamespacesPredefined), fftypes.JSONObjectArray{{"name": "default", "description": "Default predefined namespace"}}) viper.SetDefault(string(OrchestratorStartupAttempts), 5) From cc9e585e061eceee16e7b4d9e698b3d4dc935924 Mon Sep 17 00:00:00 2001 From: Peter Broadhurst Date: Thu, 10 Mar 2022 10:53:50 -0500 Subject: [PATCH 09/11] Tuning Signed-off-by: Peter Broadhurst --- internal/config/config.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/internal/config/config.go b/internal/config/config.go index a36edabbcc..279f90f909 100644 --- a/internal/config/config.go +++ b/internal/config/config.go @@ -347,8 +347,8 @@ func Reset() { viper.SetDefault(string(MessageCacheSize), "50Mb") viper.SetDefault(string(MessageCacheTTL), "5m") viper.SetDefault(string(MessageWriterBatchMaxInserts), 200) - viper.SetDefault(string(MessageWriterBatchTimeout), "200ms") - viper.SetDefault(string(MessageWriterCount), 1) + viper.SetDefault(string(MessageWriterBatchTimeout), "75ms") + viper.SetDefault(string(MessageWriterCount), 3) viper.SetDefault(string(NamespacesDefault), "default") viper.SetDefault(string(NamespacesPredefined), fftypes.JSONObjectArray{{"name": "default", "description": "Default predefined namespace"}}) viper.SetDefault(string(OrchestratorStartupAttempts), 5) From ccf89f8cb9b877e46a971e1c01ec9a530126b42f Mon Sep 17 00:00:00 2001 From: Peter Broadhurst Date: Thu, 10 Mar 2022 23:08:01 -0500 Subject: [PATCH 10/11] Review comments Signed-off-by: Peter Broadhurst --- internal/privatemessaging/message.go | 1 - 1 file changed, 1 deletion(-) diff --git a/internal/privatemessaging/message.go b/internal/privatemessaging/message.go index 39513ea3a5..6ca007ea83 100644 --- a/internal/privatemessaging/message.go +++ b/internal/privatemessaging/message.go @@ -30,7 +30,6 @@ func (pm *privateMessaging) NewMessage(ns string, in *fftypes.MessageInOut) sysm message := &messageSender{ mgr: pm, namespace: ns, - group: in.Group, msg: &data.NewMessage{ Message: in, }, From 68038c29f39489e8ac0ccf53a9dccf424168c889 Mon Sep 17 00:00:00 2001 From: Peter Broadhurst Date: Thu, 10 Mar 2022 23:09:53 -0500 Subject: [PATCH 11/11] Removed unused Signed-off-by: Peter Broadhurst --- internal/privatemessaging/message.go | 1 - 1 file changed, 1 deletion(-) diff --git a/internal/privatemessaging/message.go b/internal/privatemessaging/message.go index 6ca007ea83..6f79d1a115 100644 --- a/internal/privatemessaging/message.go +++ b/internal/privatemessaging/message.go @@ -67,7 +67,6 @@ func (pm *privateMessaging) RequestReply(ctx context.Context, ns string, in *fft type messageSender struct { mgr *privateMessaging namespace string - group *fftypes.InputGroup msg *data.NewMessage resolved bool }