Skip to content
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

ACK for every batch operation #38

Open
tsasaki87 opened this issue Nov 23, 2015 · 3 comments
Open

ACK for every batch operation #38

tsasaki87 opened this issue Nov 23, 2015 · 3 comments

Comments

@tsasaki87
Copy link

I have a question about batch operation.
Does every BatchPutKey and BatchDeleteKey request expects that server send response before Commit response?
I mean that batch operation works as follow?

CLIENT:
BatchCreate()
SERVER:
BatchCreateResp(bid)
CLIENT:
Async Put (k1,v1, bid,…)
Async Put (k2,v2, bid,…)
Async Del (k3, bid,…)

Async Put (kn,vn, bid,…)
Commit(bid)
SERVER
PutResp(k1)
PutResp(k2)
DelResp(k3)
….
PutResp(kn,vn)
CommitResp(bid)

If yes, why server need to send each separate response for BatchPut and BatchDelete?
Sending each response seems wasteful especially for large size of batch operations.

If no, memory leak should happen in kinetic-cpp-library, I think.
The library enqueues receiver handlers for each BatchPutKey/BatchDeleteKey request in NonblockingSender::Send() method, however these handlers would never be deleted because server does not send response for BatchPutKey/BatchDeleteKey.

@chiaming2000
Copy link
Contributor

Hi,

The up to date batch operation protocol has been updated to eliminate the response messages within a batch.

Kinetic/kinetic-protocol@1c52253

For example:

CLIENT:
BatchCreate(bid)
SERVER:
BatchCreateResp()
CLIENT:
Put (k1,v1, bid,…) // no response message
Put (k2,v2, bid,…) // no response message
Delete (k3, bid,…) // no response message
Put (kn,vn, bid,…) // no response message
Commit(bid)
SERVER
CommitResp(bid) // contains committed messages info., such as sequence#, etc.

Java/Python API, Simulator, and Seagate Kinetic drive implementation are conformed with the above protocol.

From your description, it seemed possible that there might be a memory leak in the kinetic-cpp-library for the batch operation implementation.

Thanks.

@tsasaki87
Copy link
Author

@chiaming2000
Thanks for your reply.
So, each BatchPutKey and BatchDeleteKey expects no response from server, correct?

And my question is why receiver handlers for BatchPutKey/BatchDeleteKey request are enqueued in NonblockingReceiver::Enqueue method?
If server does not respond for each BatchPutKey/BatchDeleteKey, thease handler would never be deleted in the "map_" variable in NonblockingReceiver class.
This situation causes that the number of entries in map_ variable would get larger with every batch operation issued from client, then memory on client-host would be exhausted in library, I think.

@chiaming2000
Copy link
Contributor

From a client's perspective, each PUT/DELETE within a batch should not expect a response message.

It is a bug if a client expects a response form a PUT/DELETE within a batch operation.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants