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.

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

AzureWebHookStore.QueryWebHooksAcrossAllUsersAsync downloads entire table into memory #12

Open
stefann42 opened this issue Mar 4, 2018 · 5 comments
Labels
bug Something isn't working

Comments

@stefann42
Copy link

stefann42 commented Mar 4, 2018

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 dougbu added this to the 1.4.0 milestone Mar 7, 2018
@dougbu
Copy link
Member

dougbu commented Mar 7, 2018

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.

@dougbu dougbu added the bug Something isn't working label Mar 7, 2018
@stefann42
Copy link
Author

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
Copy link
Member

dougbu commented Mar 9, 2018

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
Copy link
Author

stefann42 commented Mar 9, 2018

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
Copy link
Member

dougbu commented Mar 9, 2018

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

@dougbu dougbu modified the milestones: 1.4.0, 1.3.0-preview1 Mar 13, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants