fix: fix the slow xml marshaling when returning a large list of objects #1256
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
ruojunm
requested review from
will-2012,
unclezoro and
keefel
as code owners
ruojunm
commented
Nov 29, 2023
| return nil, err | ||
| } | ||
| totalObjects = append(totalObjects, objects...) | ||
| err = b.db.Table(GetObjectsTableName(bucketName)). |
There was a problem hiding this comment.
As all objects for the same buckets will only be stored into the same single sharded object table, only 1 query against GetObjectsTableName(bucketName) will be enough.
modular/gater/metadata_handler.go
Outdated
| @@ -277,14 +276,21 @@ func (g *GateModular) listObjectsByBucketNameHandler(w http.ResponseWriter, r *h | |||
| ContinuationToken: continuationToken, | |||
| } | |||
|
|
|||
| respBytes, err = xml.Marshal(grpcResponse) | |||
| if format == "json" { | |||
There was a problem hiding this comment.
Supporting JSON format in all metadata API will be implemented soon. Here just implementing the json response for one API so that later others can follow the pattern.
modular/gater/metadata_handler.go
Outdated
| @@ -611,6 +609,9 @@ func (m GfSpListObjectsByIDsResponse) MarshalXML(e *xml.Encoder, start xml.Start | |||
| } | |||
|
|
|||
| for k, v := range m.Objects { | |||
| for i, c := range v.ObjectInfo.Checksums { | |||
| v.ObjectInfo.Checksums[i] = []byte(base64.StdEncoding.EncodeToString(c)) | |||
There was a problem hiding this comment.
Before the xml marshal happens, here I encode the checksum elements into base64 encoding string and convert it to byte slice. Later the xml marshaler will still marshal []byte to string by simply invoking string([]byte) but produce the correct base64 encoded string.
| @@ -2283,7 +2278,7 @@ func (g *GateModular) getGroupMembersHandler(w http.ResponseWriter, r *http.Requ | |||
| if requestStartAfter != "" { | |||
| if ok := common.IsHexAddress(requestStartAfter); !ok { | |||
| log.Errorw("failed to check start after", "start-after", requestStartAfter, "error", err) | |||
| err = ErrInvalidHeader | |||
| err = ErrInvalidQuery | |||
| @@ -2522,7 +2513,7 @@ func (g *GateModular) listPaymentAccountStreamsHandler(w http.ResponseWriter, r | |||
|
|
|||
| if ok := common.IsHexAddress(paymentAccount); !ok { | |||
| log.Errorw("failed to check payment account", "payment-account", paymentAccount, "error", err) | |||
| err = ErrInvalidHeader | |||
| err = ErrInvalidQuery | |||
| @@ -2881,170 +2893,3 @@ func (g *GateModular) getBucketSizeHandler(w http.ResponseWriter, r *http.Reques | |||
| w.Header().Set(ContentTypeHeader, ContentTypeXMLHeaderValue) | |||
| w.Write(respBytes) | |||
| } | |||
|
|
|||
| // processObjectsXmlResponse process the unhandled Uint id and checksum of object xml unmarshal | |||
| func processObjectsXmlResponse(respBytes []byte, objects []*types.Object) (respBytesProcessed []byte) { | |||
There was a problem hiding this comment.
These code to search and replace xml content could be removed now.
BarryTong65
reviewed
Nov 29, 2023
annielz
approved these changes
Nov 29, 2023
unclezoro
reviewed
Nov 29, 2023
unclezoro
reviewed
Nov 29, 2023
unclezoro
reviewed
Nov 29, 2023
sysvm
reviewed
Nov 30, 2023
| gorm.io/driver/mysql v1.4.6 | ||
| gorm.io/gorm v1.24.5 | ||
| ) | ||
|
|
||
| require ( | ||
| google.golang.org/genproto/googleapis/api v0.0.0-20230711160842-782d3b101e98 // indirect |
unclezoro
approved these changes
Nov 30, 2023
This was referenced Dec 7, 2023
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description
fix the slow xml marshaling when returning a large list of objects
The Problem To Be Solved
The default xml marshaler does not marshal the following type:
The default xml marshaler will simply marshal the following type ([]byte) to string by invoking string([]byte).
This will leads to unreadable strings in marshaled result.
Comparing to go json marshaler, the latter by default encodes []byte as a base64-encoded string , see https://cs.opensource.google/go/go/+/refs/tags/go1.21.4:src/encoding/json/encode.go;l=56-58
And the checksums data returned by greenfield chain API (e.g. Head Object) is also encoded as base64-encoded string.
Original Solution
To resolve the above 2 issues, the original solution was to modify the default marshaled xml string by inserting Int/Uint/checksums values. It works well in most situations but will be very slow for cases with large data (e.g. query 1000 objects for a given bucket in ListObjectsByBucketName API).
New Solution in this PR
This PR improves the solution by the following 2 aspects:
Changes
Notable changes: