Fix: Update logic to calculate the maximum record size #22
Conversation
kinesis/kinesis.go
Outdated
| @@ -196,8 +196,9 @@ func (outputPlugin *OutputPlugin) AddRecord(record map[interface{}]interface{}, | |||
| } | |||
|
|
|||
| newDataSize := len(data) | |||
There was a problem hiding this comment.
Add the len(partitionKey) here so that this var is the actual "new data size"
There was a problem hiding this comment.
Also, you should error and discard the record if newDataSize > 1 MiB. That's the max size for a record. As far as I can tell we do not currently check for that in this plugin..
There was a problem hiding this comment.
Add the
len(partitionKey)here so that this var is the actual "new data size"
I intentionally kept them separated so that the next developer can easily understand how the actual dataLength is calculated.
dataLength = newDataSize + partitionKeySize
There was a problem hiding this comment.
I think you could call it newRecordSize though and they could see that the value is len(data) + len(partitionKey) which would accomplish the same thing with less code...
There was a problem hiding this comment.
Also, you should error and discard the record if
newDataSize > 1 MiB. That's the max size for a record. As far as I can tell we do not currently check for that in this plugin..
Good point. +1
I added another commit.
*Issue #21
Description of changes:
Maximum record size for kinesis stream
PutRecordsis 5mb including the size of partition keys. Earlier, we only considered the size ofdatapart of each record. This fix will stop the unexpected limit exceeds exception.By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.