aspnet / AspNetWebHooks

Libraries to create and consume web hooks on ASP.NET 4.x (Due to other priorities this project is currently in maintenance mode only. There are no planned releases at this time. No new features are planned and we are only addressing critical issues as required.)
Apache License 2.0
137 stars 103 forks source link

AzureWebHookStore.QueryWebHooksAcrossAllUsersAsync downloads entire table into memory #12

Open stefann42 opened 6 years ago

stefann42 commented 6 years ago

From reading the code it seems this library downloads the entire content of the Azure Table to memory in order to query existing web hooks for ones which match specific actions. Is that correct?

var query = new TableQuery();
var entities = await _manager.ExecuteQueryAsync(table, query);

Has this been tested at scale i.e. is this used by any service with 1000+ active web hooks? As the table content grows, downloading and deserializing the entire table in memory can get pretty slow and eat up a lot of memory. There are also $ costs associated with table operations and network bandwidth at scale. Has that factor been taken into consideration?

Thanks, just trying to understand.

dougbu commented 6 years ago

Thanks for reporting this issue. We agree that this seems to be a potential bug and have placed it in the next milestone.

Do you have data indicating this is more than a potential problem? It seems AzureWebHookStore.DeleteWebHookAsync(...) and DeleteAllWebHooksAsync(...) calls would need to fall way behind InsertWebHookAsync(...) calls for memory issues to surface.

stefann42 commented 6 years ago

I'm not sure I understand what you mean by "fall behind". Are you saying that in order for this issue to be a problem in production the rate of inserts would have to be higher than the rate of deletes? A webhook doesn't have an intrinsic expiration so it will exist as long as the user wants the webhook subscription, no?

FWIW, when I said "memory" I was referring to RAM required to load and deserialize the data on the web server, not storage space in Table Storage (which is negligible from a cost perspective).

dougbu commented 6 years ago

Are you saying that in order for this issue to be a problem in production the rate of inserts would have to be higher than the rate of deletes?

Yes

A webhook doesn't have an intrinsic expiration so it will exist as long as the user wants the webhook subscription, no?

The table will grow slowly as users increase their webhook count and as more users are added.

stefann42 commented 6 years ago

It's not going to grow that slowly if you introduce this feature in an existing SaaS product with existing customers who have been asking for it, which is what I'm trying to do :). Imagine if an existing Microsoft product adopts this library. The first thing they'd do is change the schema of this storage provider.

The rate of growth isn't really a relevant point unless you guys designed this library only for devs who are prototyping or building MVPs. If I know I'll have 1000+ active webhooks it doesn't make me feel better that I'll hit that number in say 12 months vs. 3 months. Adopting new schema with scalability issues means you're signing up for a major schema change at some point in the future. It's preferable to do it right from the start (unless, of course, you are building a prototype/MVP).

dougbu commented 6 years ago

For your scenario, I'd recommend subclassing AzureWebHookStore and changing the application to use your subclass.