[v2] Encoder server with EncodeBlob method #866
Conversation
| case s.requestPool <- struct{}{}: | ||
| default: | ||
| // TODO: Now that we no longer pass the data directly, should we pass in blob size as part of the request? | ||
| s.metrics.IncrementRateLimitedBlobRequestNum(1) |
There was a problem hiding this comment.
Previously we were passing blobSize, not sure if we want to keep that behavior.
disperser/cmd/encoder/main.go
Outdated
| logger.Info("Blob store", "bucket", blobStoreBucketName) | ||
|
|
||
| chunkStoreBucketName := config.ChunkStoreConfig.BucketName | ||
| chunkWriter := chunkstore.NewChunkWriter(logger, s3Client, chunkStoreBucketName, 4*1024*1024) // TODO(dmanc): make fragment size configurable? |
There was a problem hiding this comment.
Not a bad thing to make configurable. That being said, this is unlikely to be something we change often (if ever)... so possibly acceptable to hard code it in the short term.
There was a problem hiding this comment.
Sounds good, I'll leave it hardcoded and update the comment.
|
|
||
| "github.com/Layr-Labs/eigenda/common/aws/s3" | ||
| v2 "github.com/Layr-Labs/eigenda/core/v2" |
There was a problem hiding this comment.
Is the v2 alias necessary? Isn't the package name already v2?
There was a problem hiding this comment.
Going to alias it to corev2 for clarity.
There was a problem hiding this comment.
For some reason the linter didn't like changing it for one specific file types_test.go https://github.com/Layr-Labs/eigenda/actions/runs/11711503781/job/32620653412
|
|
||
| // Rate limit | ||
| select { | ||
| case s.requestPool <- struct{}{}: |
There was a problem hiding this comment.
do you think this is still a good way to rate limit requests?
There was a problem hiding this comment.
I'll discuss with @jianoaix when he gets back but leaving it as is for now.
| @@ -23,3 +23,25 @@ func NextPowerOf2[T constraints.Integer](d T) T { | |||
| nextPower := math.Ceil(math.Log2(float64(d))) | |||
| return T(math.Pow(2.0, nextPower)) | |||
| } | |||
|
|
|||
| // PadToPowerOf2 pads a byte slice to the nearest power of 2 length by appending zeros | |||
| func PadToPowerOf2(data []byte) []byte { | |||
There was a problem hiding this comment.
@mooselumph can you confirm if this is the correct way to enforce the number of chunks to be power of 2?
There was a problem hiding this comment.
This is enforcing the number of bytes being a power of two rather than the number of chunks, which isn't what we would want.
I'm also not sure that such a function should be needed. We just need to make sure that the Length claimed by the client in the BlobHeader is appropriately rounded up to the next power of two.
| package encoder.v2; | ||
|
|
||
| service Encoder { | ||
| rpc EncodeBlobToChunkStore(EncodeBlobToChunkStoreRequest) returns (EncodeBlobToChunkStoreReply) {} |
There was a problem hiding this comment.
Describe what does this API do?
Also should it be just EncodeBlob? Is storing the results to chunk store an important thing to call out in the name?
There was a problem hiding this comment.
Changed it back to EncodeBlob. I think if we ever have a desire to have an endpoint that immediately returns the encoded data then we would need to add more information in the name.
|
|
||
| // Rate limit | ||
| select { | ||
| case s.requestPool <- struct{}{}: |
Why are these changes needed?
Implements V2 server with the
EncodeBlobmethod.Checks