From ffe1a285bdc46e0238433bb177b23305d312e931 Mon Sep 17 00:00:00 2001 From: Todd Gruben Date: Mon, 18 Feb 2019 16:28:37 -0600 Subject: [PATCH 1/4] removed copy for pilosa roaring files --- api.go | 20 +++++++++++++++----- roaring/roaring.go | 14 +++++++------- 2 files changed, 22 insertions(+), 12 deletions(-) diff --git a/api.go b/api.go index d3fb03615..17c01637b 100644 --- a/api.go +++ b/api.go @@ -18,6 +18,7 @@ package pilosa import ( "context" + "encoding/binary" "encoding/csv" "fmt" "io" @@ -324,11 +325,20 @@ func (api *API) ImportRoaring(ctx context.Context, indexName, fieldName string, } // must make a copy of data to operate on locally. // field.importRoaring changes data - data := make([]byte, len(viewData)) - copy(data, viewData) - err = field.importRoaring(data, shard, viewName, req.Clear) - if err != nil { - return err + fileMagic := uint32(binary.LittleEndian.Uint16(viewData[0:2])) + if fileMagic == roaring.MagicNumber { // if pilosa roaring + err = field.importRoaring(viewData, shard, viewName, req.Clear) + if err != nil { + return err + } + + } else { + data := make([]byte, len(viewData)) + copy(data, viewData) + err = field.importRoaring(data, shard, viewName, req.Clear) + if err != nil { + return err + } } } return err diff --git a/roaring/roaring.go b/roaring/roaring.go index abc4e4d2f..a34d9347e 100644 --- a/roaring/roaring.go +++ b/roaring/roaring.go @@ -28,15 +28,15 @@ import ( ) const ( - // magicNumber is an identifier, in bytes 0-1 of the file. - magicNumber = uint32(12348) + // MagicNumber is an identifier, in bytes 0-1 of the file. + MagicNumber = uint32(12348) // storageVersion indicates the storage version, in bytes 2-3. storageVersion = uint32(0) // cookie is the first four bytes in a roaring bitmap file, - // formed by joining magicNumber and storageVersion - cookie = magicNumber + storageVersion<<16 + // formed by joining MagicNumber and storageVersion + cookie = MagicNumber + storageVersion<<16 // headerBaseSize is the size in bytes of the cookie and key count at the // beginning of a file. @@ -935,10 +935,10 @@ func (b *Bitmap) unmarshalPilosaRoaring(data []byte) error { return errors.New("data too small") } - // Verify the first two bytes are a valid magicNumber, and second two bytes match current storageVersion. + // Verify the first two bytes are a valid MagicNumber, and second two bytes match current storageVersion. fileMagic := uint32(binary.LittleEndian.Uint16(data[0:2])) fileVersion := uint32(binary.LittleEndian.Uint16(data[2:4])) - if fileMagic != magicNumber { + if fileMagic != MagicNumber { return fmt.Errorf("invalid roaring file, magic number %v is incorrect", fileMagic) } @@ -4020,7 +4020,7 @@ func (b *Bitmap) UnmarshalBinary(data []byte) error { } statsHit("Bitmap/UnmarshalBinary") fileMagic := uint32(binary.LittleEndian.Uint16(data[0:2])) - if fileMagic == magicNumber { // if pilosa roaring + if fileMagic == MagicNumber { // if pilosa roaring return errors.Wrap(b.unmarshalPilosaRoaring(data), "unmarshaling as pilosa roaring") } From a13e5fafa45c38c262b70ba2bd3663711ac35ead Mon Sep 17 00:00:00 2001 From: Todd Gruben Date: Tue, 19 Feb 2019 10:19:17 -0600 Subject: [PATCH 2/4] updated comments and added error wrapping --- api.go | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/api.go b/api.go index 17c01637b..eae4283c4 100644 --- a/api.go +++ b/api.go @@ -323,21 +323,21 @@ func (api *API) ImportRoaring(ctx context.Context, indexName, fieldName string, if len(viewData) == 0 { return fmt.Errorf("no data to import for view: %s", viewName) } - // must make a copy of data to operate on locally. - // field.importRoaring changes data fileMagic := uint32(binary.LittleEndian.Uint16(viewData[0:2])) - if fileMagic == roaring.MagicNumber { // if pilosa roaring + if fileMagic == roaring.MagicNumber { // if pilosa roaring format err = field.importRoaring(viewData, shard, viewName, req.Clear) if err != nil { - return err + return errors.Wrap(err,"import pilosa roaring") } } else { + // must make a copy of data to operate on locally on standard roaring format. + // field.importRoaring changes the standard roaring run format to pilosa roaring data := make([]byte, len(viewData)) copy(data, viewData) err = field.importRoaring(data, shard, viewName, req.Clear) if err != nil { - return err + return errors.Wrap(err,"import standard roaring") } } } From 02ed4568dd34a4e274df4b9543c5383d8bbb4a69 Mon Sep 17 00:00:00 2001 From: Todd Gruben Date: Tue, 19 Feb 2019 10:25:34 -0600 Subject: [PATCH 3/4] gofmt --- api.go | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/api.go b/api.go index eae4283c4..5812daca0 100644 --- a/api.go +++ b/api.go @@ -327,17 +327,17 @@ func (api *API) ImportRoaring(ctx context.Context, indexName, fieldName string, if fileMagic == roaring.MagicNumber { // if pilosa roaring format err = field.importRoaring(viewData, shard, viewName, req.Clear) if err != nil { - return errors.Wrap(err,"import pilosa roaring") + return errors.Wrap(err, "import pilosa roaring") } } else { - // must make a copy of data to operate on locally on standard roaring format. - // field.importRoaring changes the standard roaring run format to pilosa roaring + // must make a copy of data to operate on locally on standard roaring format. + // field.importRoaring changes the standard roaring run format to pilosa roaring data := make([]byte, len(viewData)) copy(data, viewData) err = field.importRoaring(data, shard, viewName, req.Clear) if err != nil { - return errors.Wrap(err,"import standard roaring") + return errors.Wrap(err, "import standard roaring") } } } From e3fe55522d5da3c61fd7ee2050e7744c2496b736 Mon Sep 17 00:00:00 2001 From: Todd Gruben Date: Tue, 19 Feb 2019 10:54:09 -0600 Subject: [PATCH 4/4] comment adjustments --- api.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/api.go b/api.go index 5812daca0..fc4ca41c8 100644 --- a/api.go +++ b/api.go @@ -327,7 +327,7 @@ func (api *API) ImportRoaring(ctx context.Context, indexName, fieldName string, if fileMagic == roaring.MagicNumber { // if pilosa roaring format err = field.importRoaring(viewData, shard, viewName, req.Clear) if err != nil { - return errors.Wrap(err, "import pilosa roaring") + return errors.Wrap(err, "importing pilosa roaring") } } else { @@ -337,7 +337,7 @@ func (api *API) ImportRoaring(ctx context.Context, indexName, fieldName string, copy(data, viewData) err = field.importRoaring(data, shard, viewName, req.Clear) if err != nil { - return errors.Wrap(err, "import standard roaring") + return errors.Wrap(err, "importing standard roaring") } } }