unrecognized paths OOM fix #746
Conversation
| @@ -25,5 +25,6 @@ public class Config extends com.uid2.shared.Const.Config { | |||
| public static final String GcpSecretVersionNameProp = "gcp_secret_version_name"; | |||
| public static final String OptOutStatusApiEnabled = "optout_status_api_enabled"; | |||
| public static final String OptOutStatusMaxRequestSize = "optout_status_max_request_size"; | |||
| public static String MaxInvalidPaths = "logging_limit_max_invalid_paths_per_interval"; | |||
| @@ -113,7 +116,11 @@ public void handleMessage(Message message) { | |||
|
|
|||
| EndpointStat endpointStat = new EndpointStat(endpoint, siteId, apiVersion, domain); | |||
|
|
|||
| pathMap.merge(path, endpointStat, this::mergeEndpoint); | |||
| Set<String> endpoints = Endpoints.pathSet(); | |||
| if(endpoints.contains(path) || pathMap.containsKey(path) || (pathMap.size() < this.maxInvalidPaths + endpoints.size() && messageItem.getApiContact() != null)) { | |||
There was a problem hiding this comment.
There is a lot going on in this if. Can we break it up a little to make it easier to read?
| @@ -0,0 +1,59 @@ | |||
| package com.uid2.operator.vertx; | |||
There was a problem hiding this comment.
Is there a way we can autogenerate this? I am worried people will forget to add new paths here. I think vertx has a list of all our paths in it after we add them all to the router
There was a problem hiding this comment.
right, there's a getRoutes() method, hopefully we can use that rather than specifying the paths twice
There was a problem hiding this comment.
I have been able to use the getRoutes method but it requires us to create and dispose of a verticle to retrieve the path definition. This is because the route definition has to be associated with a particular instance of the verticle (we deploy 8 of them but only 1 StatsCollectorVerticle and only 1 setupMetrics()) because we need to use the methods of the class, the auth middleware, etc in the route definition. They cant be defined statically. We also then have to work around the healthcheck as we never deploy the instance of the verticle we use to retrieve the routes in Main.java, and so never mark the HTTP server as started. This commit shows the setup with getRoutes() 988ea51
For this reason, I prefer the ENUM. The strings of the paths are only defined in one place, and since the routes are all defined together in createRoutesSetup() people will see the ENUM being used and know to define the string there. Similar to how we define our config keys in Const.Config.Java and people adhere to it and define their keys there.
There was a problem hiding this comment.
There is also the option to send the paths from the operator verticle instances to the StatsCollectorVerticle over the event bus, but I still prefer the ENUM to this.
No description provided.