fix: remove monotonic_timestamp#526
Open
thayinm wants to merge 2 commits into
Open
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description of changes:
A previous pull request introduced the
_get_monotonic_timestampfunction in order to address the issue of out of order tool calls which can lead to a ValidationException with the Converse API. However this change did not address the issue as users are still able to encounter the ValidationException under certain circumstances.By using a 1 second resolution with an Agent that can run many tool calls, it will result in timestamps generated many seconds into the future. If the same Agent is being used then this is not an issue as that Agent will still have the proper
_last_timestamppreserved so it will continue to increment on that but if an Agent is hosted behind a cluster such as EKS where requests that come in with a session_id do not route to the same pod/container. The initial timestamp generated in a new pod (where a new instance of theAgentCoreMemorySessionManagerclass will happen) is going to be the current timestamp which can be less than the last timestamp of the last record in AgentCore Memory if the API calls happen within a very short period of time.The solution here is to revert back to the previous method of recording timestamps (which is to use
datetime.now(timezone.utc)). Since timestamps are generated down to the microsecond level (this may have not been the case when the prior pull request was created but from testing directly with the boto3 sdk timestamps are at a microsecond granularity) the likelihood that 2 messages will share the exact same time at this resolution is effectively zero for a given session.Testing
The easiest way to replicate the issue is to run the below
test.pyfile multiple times in quick succession with a model that has very low latency. The below bash script does not create a Memory if it does not exist so you will have to manually create this.Bash Script
Python Script