-
Notifications
You must be signed in to change notification settings - Fork 487
Use State to track the order of MongoDB transactions #1742
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
Use State to track the order of MongoDB transactions #1742
Conversation
ddelnano
left a comment
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'm still in the process of reviewing the stitcher change. I want to understand how this is leveraged fully before giving the final review, but here are some additional comments I had.
src/stirling/source_connectors/socket_tracer/protocols/mongodb/parse_test.cc
Outdated
Show resolved
Hide resolved
src/stirling/source_connectors/socket_tracer/protocols/mongodb/parse_test.cc
Outdated
Show resolved
Hide resolved
src/stirling/source_connectors/socket_tracer/protocols/mongodb/types.h
Outdated
Show resolved
Hide resolved
ddelnano
left a comment
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.
One more minor comment, but otherwise lgtm!
src/stirling/source_connectors/socket_tracer/protocols/mongodb/types.h
Outdated
Show resolved
Hide resolved
Signed-off-by: Kartik Pattaswamy <kpattaswamy@pixielabs.ai>
Signed-off-by: Kartik Pattaswamy <kpattaswamy@pixielabs.ai>
Signed-off-by: Kartik Pattaswamy <kpattaswamy@pixielabs.ai>
Signed-off-by: Kartik Pattaswamy <kpattaswamy@pixielabs.ai>
9aed0ae to
db9c67a
Compare
|
Could you expand the PR description to explain what the underlying issue is/was and how the state helps address the issue? Also, could you describe the test case in more detail (i.e. in the PR description). Something like: "the test case parses 3 frames. The first frame is a request with XYZ characteristics. After parsing that frame, the test expects to find the state vector in this specific form because X." |
Signed-off-by: Kartik Pattaswamy <kpattaswamy@pixielabs.ai>
Summary: This PR adds functionality to track the order of transactions as they are parsed. It adds the streamID of each request to the state vector at parsing time which will then be used to iterate over at stitching time. Adding this will mainly help the stitching process when trying to stitch a request with N
moreToComeresponses.Motivation behind this change:
The stitching implementation relies on the new interface using
absl::flat_hash_mapto store thestreamIDand a deque of request/response frames. We then use a response led matching algorithm where we loop through the response map and stitch the first response frame in a deque with its corresponding request frame. A response pairs with a request when both frames share the samestreamID, the response frame'sstreamIDis itsresponseToand the request frame'sstreamIDis itsrequestID.MongoDB's
OP_MSGwire protocol has the concept ofmore_to_comewhich means that the server could sendNresponses back to a singular request by the client. Each frame in the series of theNresponses are linked by theresponseToof the frame matching therequestIDof the previous response frame, similar to a singly linked list. The head response frame'sresponseTowill be therequestIDof the request frame. Note: therequestIDof each frame in theN more_to_comeframes is random and unique.At the time of stitching, if we do not use state to track the order of transactions we would iterate over the response map in a "random" order and could iterate over the
more_to_comeresponse frames out of order. We could lose context on how themore_to_comeframes are linked due to not knowing the head response frame and if we were to iterate over the end of themore_to_comemessage before looping through all priormore_to_comeframes in the message they would be dropped since we do not know which request those frames are responding to. To solve this issue, tracking the order of transactions'streamIDsto iterate over would ensure that could use the response led stitching approach and find the completemore_to_comemessage for a given request.New test case:
The new test case checks to make sure the state's
stream_ordervector is correctly populated with the order ofstreamIDsas we parse new request frames (transactions). The test case parses 3 frames and expects that the state'sstream_orderafter parsing the first frame to containstd::pair<917, false>since the first frame'srequestIDis 917. It expects thestream_orderto containstd::pair<917, false>,std::pair<444, false>after parsing the second request frame since that frame's requestID is 444 and so on.Related issues: #640
Type of change: /kind feature
Test Plan: Modified the existing tests and added another test to make sure the vector is populated correctly.