New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Dynamo use gloabal worker #624
Conversation
Co-authored-by: Melvin Junhee Woo <melvin.woo@groundx.xyz>
… into dynamo-gloabal-worker
4cf0736
to
281ed4a
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
281ed4a
to
4a497a0
Compare
4a497a0
to
b15e8af
Compare
@@ -474,7 +487,7 @@ func (batch *dynamoBatch) Put(key, val []byte) error { | |||
|
|||
if len(batch.batchItems) == dynamoBatchSize { | |||
batch.wg.Add(1) | |||
batch.db.writeCh <- &batchWriteWorkerInput{batch.batchItems, batch.wg} | |||
dynamoWriteCh <- &batchWriteWorkerInput{batch.tableName, batch.batchItems, batch.wg} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is there any possibility of panic if Close
is called while processing Put
operation?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've just tested.
A panic will not occur even though Close()
is called on non-empty channel. (call Put()
-> Close()
) It will close after all of the items are processed.
However, a panic will occur when put is called in a closed channel (call Close()
-> Put()
).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes. To avoid the second case, we need to use switch & case. Do you have any other alternatives?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If we use the original code, not all of the items in the channel will be processed. There might be some items we expect to be writen but actually no. Actually, in the original code, it will wait inifinitively in Write()
because the workers will not process the input and wg.Done()
is not called.
Is there other ways to handle both cases?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we don't need to consider the 2nd case. It is misbehavior of users.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we don't need to consider the 2nd case. It is misbehavior of users.
I think this is the case which rarely happens.
However, I don't think it is misbehavior of users. Since the user does not know whether it will cause panic or not.
However, again, as Database.Close
is called after Blockchain.Stop
, there is no possibility that Database.Put
is called after Database.Close
.
Proposed changes
dynamo-table
, tables such asdynamo-table-body
,dynamo-table-header
,dynamo-table-statetrie
, and so on will be created.Types of changes
Please put an x in the boxes related to your change.
Checklist
Put an x in the boxes that apply. You can also fill these out after creating the PR. If you're unsure about any of them, don't hesitate to ask. We're here to help! This is simply a reminder of what we are going to look for before merging your code.
$ make test
)Related issues
Further comments