Add support to batch insert messages#7
Add support to batch insert messages#7MohamedSabthar merged 11 commits intoballerina-platform:mainfrom
Conversation
…andled my Memory Implementations
MohamedSabthar
dismissed
SasinduDilshara’s stale review
The merge-base changed after approval.
MaryamZi
left a comment
MaryamZi
left a comment
There was a problem hiding this comment.
Some comments from the other PR apply here too.
| return error("Failed to load message count from the database: " + messageCount.message(), messageCount); | ||
| } | ||
|
|
||
| if messageCount > self.maxMessagesPerKey { |
There was a problem hiding this comment.
We do't need to do that. It should be handled by the caller as discussed previously with @shafreenAnfar and @SasinduDilshara.
There was a problem hiding this comment.
For updates, yes. But this gets called during a get too, right? I'm not sure we can expect users to do this check for gets also.
There was a problem hiding this comment.
if updates are already handled by the user, that guarantees the memory will always have a message count below the configured size. I do not think we need to handle this in get.
There was a problem hiding this comment.
Won't happen for the first get that happens if no update has happened, right?
There was a problem hiding this comment.
As discussed offline with @MaryamZi, there can be cases where a memory instance is created with a specific size configuration, for example 10, and updates are persisted accordingly, so the database contains 10 entries. Later, if the user code is changed to use a smaller size, the database may still hold more entries than the new limit, which could result in an error.
Since this is a very rare scenario, we are currently removing the size check to reduce database calls.
There was a problem hiding this comment.
When the cache is used, there'll be less database calls anyway, so one here would be okay IMO.
3 participants
Purpose
$subject
Examples
Related to: wso2/product-ballerina-integrator#2081, wso2/product-ballerina-integrator#2110
Checklist