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.

Sign up for GitHub

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

Jump to bottom

:MessageSystemAttributeName spec uses strings, and the result is keywords #50

Closed
danieroux opened this issue Jan 25, 2019 · 1 comment
Closed

:MessageSystemAttributeName spec uses strings, and the result is keywords #50

danieroux opened this issue Jan 25, 2019 · 1 comment
Labels
bug Something isn't working

Comments

@danieroux
Copy link 

(s/explain :cognitect.aws.sqs/ReceiveMessageResult
           (aws/invoke sqs {:op      :ReceiveMessage
                            :request (merge queue-url {:AttributeNames ["All"]})}))

Fails spec with:

In: [:Messages 0 :Attributes :SenderId 0] val: :SenderId fails spec: :cognitect.aws.sqs/MessageSystemAttributeName at: [:Messages :Attributes 0] predicate: #{"SentTimestamp" "SenderId" "SequenceNumber" "ApproximateReceiveCount" "MessageGroupId" "ApproximateFirstReceiveTimestamp" "MessageDeduplicationId"}
In: [:Messages 0 :Attributes :ApproximateFirstReceiveTimestamp 0] val: :ApproximateFirstReceiveTimestamp fails spec: :cognitect.aws.sqs/MessageSystemAttributeName at: [:Messages :Attributes 0] predicate: #{"SentTimestamp" "SenderId" "SequenceNumber" "ApproximateReceiveCount" "MessageGroupId" "ApproximateFirstReceiveTimestamp" "MessageDeduplicationId"}
In: [:Messages 0 :Attributes :ApproximateReceiveCount 0] val: :ApproximateReceiveCount fails spec: :cognitect.aws.sqs/MessageSystemAttributeName at: [:Messages :Attributes 0] predicate: #{"SentTimestamp" "SenderId" "SequenceNumber" "ApproximateReceiveCount" "MessageGroupId" "ApproximateFirstReceiveTimestamp" "MessageDeduplicationId"}
In: [:Messages 0 :Attributes :SentTimestamp 0] val: :SentTimestamp fails spec: :cognitect.aws.sqs/MessageSystemAttributeName at: [:Messages :Attributes 0] predicate: #{"SentTimestamp" "SenderId" "SequenceNumber" "ApproximateReceiveCount" "MessageGroupId" "ApproximateFirstReceiveTimestamp" "MessageDeduplicationId"}

The received message has this structure:

{:Messages
 [{:MessageId "xxx",
   :ReceiptHandle "xxx",
   :MD5OfBody "0a997fa7fd98d3850f5ce76c943afe7c",
   :Body "xxx",
   :Attributes
   {:SenderId "xxx",
    :ApproximateFirstReceiveTimestamp "1548363122352",
    :ApproximateReceiveCount "11",
    :SentTimestamp "1548363122352"}}]}
@dchelimsky dchelimsky added the bug Something isn't working label Jan 25, 2019
@dchelimsky
Copy link
Contributor

Fixed on master.

scottbale pushed a commit that referenced this issue Dec 23, 2024
@marcobiscaro2112
Performance improvements in highly parallel scenarios (#50)
1bcd020 
Problem

In applications with a high number of parallel operations in multiple threads, we measured some performance hot spots:
- date and time formatting and parsing, mainly caused by the ThreadLocal instances around java.text.SimpleDateFormat (that is not thread safe)
- URI encoding when signing requests

Solution

- Use thread-safe a java.time.format.DateTimeFormatter instance, instead of instance-per-thread of SimpleDateFormat using ThreadLocal's
- Tweak uri-encode to avoid the creation of many temporary LazySeq instances, and only compute the fixed set of safe characters once (instead of on every invocation)

Rationale

DateTimeFormatter is thread safe (and more correct), but since the API in aws-api handles java.util.Date instances exclusively, this requires an additional transformation of java.util.Date -> java.time.ZonedDateTime when formatting (and the other way around when parsing). This new transformation should not impact performance negatively, and it avoids a much more expensive object creation of SimpleDateFormat instances.

This change also fixes a latent bug, that returns the wrong parsed timestamp if the remote API includes a ISO 8601 string with more precision than milliseconds. The parser would wrongly consider anything after the `.` as milliseconds, even if there are 6 digits (that represent in fact microseconds). This bug causes parsed timestamp to be off by some minutes

```
; with 2023-01-23T11:59:03.575496Z as input, the old parser considers
; the timestamp has 575496ms, which is obviously wrong (instead of
; truncating to 575ms as the new parser correctly does)
(.plus (Instant/parse "2023-01-23T11:59:03Z") (Duration/ofMillis 575496))
=> #object[java.time.Instant 0x293edc20 "2023-01-23T12:08:38.496Z"]
```
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
bug Something isn't working
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
2 participants
@dchelimsky @danieroux