search

ericblade / mws-advanced

Javascript (node) Amazon Merchant Web Services (MWS) code with mws-simple as it's underpinnings
Apache License 2.0
41 stars 11 forks source link

throttle: need a mechanism to define calls that have shared quotas #109

Open ericblade opened 5 years ago

ericblade commented 5 years ago

example: GetReportList and GetReportListByNextToken share the same request quota. Lib does not currently understand that. Probably need to include a "throttle key" option in the throttle data that different calls can use to mark that they should all belong to the same Queue.

pwrnrd commented 5 years ago

What about the throttle details returned by the API? I recall an issue we had before, where you updated MWS-Simple to pass along the throttling headers, right?

Would it be an idea to keep count of the request send and to check these against the counts received from the MWS API (through the headers)? Depending on the difference between the local count and the count retrieved from the MWS API we can create an estimate of the amount of processes that run simultaneously. Subsequently, we can multiply the retry time by the amount of processes.

Method:

  1. Send request to MWS API
  2. Locally update the count of the remaining quota for the request send.
  3. Retrieve MWS API response header x-mws-quota-remaining
  4. Compare local count of remaining request with x-mws-quota-remaining.
  5. If there is a non-zero difference between the local count and x-mws-quota-remaining, adjust the request retry time.

Could this approach work? Or perhaps there is a better method to implement this?

ericblade commented 5 years ago

yep, it should pass along the throttle headers now, so we can be more smart with them, but we can't rely on them 100% -- say you've got two completely separate instances of your service running (as I do), they don't currently know about each other's quota usages at all, so one could make a request, be completely declined, and have no idea why, and no headers to say when to try again, because throttle errors don't appear to provide headers, which is frankly, dumb. But that's what we have (unless there's an error in simple that i didn't catch)

Queue or QueueItem should be able to inspect whatever throttle control headers are present and alter themselves appropriately. I think we'll still need to add a bit to deal with items that share quotas, though, if another instance of the app, or another app completely externally, is issuing requests against your quotas, perhaps the first request this app makes gets a throttle error, and doesn't receive any headers.

pwrnrd commented 5 years ago

you've got two completely separate instances of your service running (as I do), they don't currently know about each other's quota usages at all

Exactly! Therefore, I was thinking about the information both of these instances should know, and that's the MWS response they retrieve. So I thought that we could use that information to scale the time between requests. However, this might also be bit overkill... In addition, the problem you mention regarding the quota headers not being send is tricky in case of multiple instances/applications. Therefore, I decided to rewrite my current implementation.

I think adding a throttle key as you describe makes sense for the simpler cases (shared quotas/single app with one or multiple instances).

EDIT: This might be a good first issue for someone to pick up.