Rate-limits with the JDBC plugin and PostgreSQL don't work

clement-is commented 2 years ago

Current Behavior

When I configure the rate-limits via a policy and I request with simultaneous calls via Jmeter on my API (with a keyless plan), the Gravitee Gateway doesn't limit the number of requests received by the API. This behavior appears when I use the JDBC connector with PostgreSQL. Otherwise with the MongoDB default connection, I don't have this problem.

Steps to Reproduce (for bugs)

https://github.com/clement-dev/gravitee-issue-jdbc.git
Set a rate-limits: 20 rates / minutes
With JMeter, set a Thread Group with 10 users & Loop count 50

Your Environment

Version JDBC driver: JDBC postgresql-42.2.18.jar Docker Image Api mangement: graviteeio/apim-management-api:3Docker Image Gateway: graviteeio/apim-gateway:3 Java version: 11.0.13

phiz71 commented 2 years ago

Hello @clement-dev, I tried to start your docker-compose, but I noticed lots of error in the gateway & in the management API logs. You need to create first a gravitee DB and a user. You can check this stackoverflow page for this: https://stackoverflow.com/a/26599273

If you still reproduce the problem after that, could you also join the definition of the API you used to test the ratelimit ? Thank you 🙏🏻

phiz71 commented 2 years ago

Ok I've seen you've already prepared stuffs to init the db 👍🏻 . But your init.sql file is empty.

clement-is commented 2 years ago

Thanks for your quick response. Indeed, my init.sql file is empty but that's okay because the Gravitee DB is created with env variables in docker-compose: POSTGRES_DB: gravitee POSTGRES_USER: admin POSTGRES_PASSWORD: admin I manage to connect with Datagrip... You will find the export of the API configuration below:

export: 3.12.1

{
  "name": "test",
  "version": "test",
  "description": "https://jsonplaceholder.typicode.com/todos/1",
  "visibility": "PUBLIC",
  "gravitee": "2.0.0",
  "flow_mode": "DEFAULT",
  "resources": [],
  "properties": [],
  "members": [
    {
      "source": "memory",
      "sourceId": "admin",
      "roles": [
        "9a278433-ea75-4137-a784-33ea7591373b"
      ]
    }
  ],
  "pages": [
    {
      "id": "13b164e7-f55d-4192-b164-e7f55dd1924d",
      "name": "Aside",
      "type": "SYSTEM_FOLDER",
      "order": 0,
      "published": true,
      "visibility": "PUBLIC",
      "lastModificationDate": 1637007825070,
      "contentType": "application/json",
      "configuration": {},
      "homepage": false,
      "parentPath": "",
      "excludedAccessControls": false,
      "accessControls": [],
      "metadata": {},
      "api": "2b687f18-97ce-4826-a87f-1897ce3826ed",
      "attached_media": []
    }
  ],
  "plans": [
    {
      "id": "c414c744-b90e-4cbc-94c7-44b90e4cbc06",
      "name": "free",
      "description": "test",
      "validation": "AUTO",
      "security": "KEY_LESS",
      "type": "API",
      "status": "PUBLISHED",
      "api": "2b687f18-97ce-4826-a87f-1897ce3826ed",
      "order": 0,
      "characteristics": [],
      "tags": [],
      "created_at": 1637007825156,
      "updated_at": 1637007825156,
      "paths": {},
      "flows": [
        {
          "name": "",
          "path-operator": {
            "path": "/",
            "operator": "STARTS_WITH"
          },
          "condition": "",
          "consumers": [],
          "methods": [],
          "pre": [
            {
              "name": "Rate limit",
              "description": "",
              "enabled": true,
              "policy": "rate-limit",
              "configuration": {
                "rate": {
                  "limit": 20,
                  "periodTime": 1,
                  "periodTimeUnit": "MINUTES"
                }
              }
            }
          ],
          "post": [],
          "enabled": true
        }
      ],
      "excluded_groups": [],
      "comment_required": false
    }
  ],
  "metadata": [
    {
      "key": "email-support",
      "name": "email-support",
      "format": "MAIL",
      "value": "${(api.primaryOwner.email)!''}",
      "defaultValue": "support@change.me",
      "apiId": "2b687f18-97ce-4826-a87f-1897ce3826ed"
    }
  ],
  "id": "2b687f18-97ce-4826-a87f-1897ce3826ed",
  "path_mappings": [],
  "proxy": {
    "virtual_hosts": [
      {
        "path": "/test"
      }
    ],
    "strip_context_path": false,
    "preserve_host": false,
    "groups": [
      {
        "name": "default-group",
        "endpoints": [
          {
            "name": "default",
            "target": "https://jsonplaceholder.typicode.com/todos/1",
            "weight": 1,
            "backup": false,
            "type": "HTTP",
            "inherit": true
          }
        ],
        "load_balancing": {
          "type": "ROUND_ROBIN"
        },
        "http": {
          "connectTimeout": 5000,
          "idleTimeout": 60000,
          "keepAlive": true,
          "readTimeout": 10000,
          "pipelining": false,
          "maxConcurrentConnections": 100,
          "useCompression": true,
          "followRedirects": false
        }
      }
    ]
  },
  "response_templates": {},
  "primaryOwner": {
    "id": "4020233c-3772-460f-a023-3c3772260f8a",
    "displayName": "admin",
    "type": "USER"
  }
}

There is a lot of data in postgres, however the table "ratelimit" is empty. Thanks again for your help 🙏🏻

phiz71 commented 2 years ago

It seems that you have not configured ratelimit to also use postgres. https://github.com/clement-dev/gravitee-issue-jdbc/blob/main/config/gravitee/gateway/config/gravitee.yml#L143-L146

You can have a look here to help you: https://docs.gravitee.io/apim/3.x/apim_installguide_gateway_configuration.html#store_counters_in_jdbc

mouligno commented 2 years ago

Can we close it ?

clement-is commented 2 years ago

Sorry for the late answer. I fixed my configuration but the issue persists. I also updated my template here: https://github.com/clement-dev/gravitee-issue-jdbc In the readme, you will find my test strategy which makes me say that it doesn't work. Thank you in advance, 🙏

phiz71 commented 2 years ago

Hello @clement-dev, thank you for your contribution. We will have a look soon at your PR.

Could you just remind me which version of Gravitee you tested ? Thanks 🙏🏻

clement-is commented 2 years ago

Your welcome ! Version 3.15.4 👍🏻

marcambier commented 2 years ago

Hello @clement-dev. Thanks again for your contribution. I will have a look at you PR this week, and let you know about that.

marcambier commented 2 years ago

Hello @clement-dev

I confirm :

I'm able to reproduce the problem using the way you described
I agree there is an inconsistency with JDBC rate limit repository, caused by SQL transactions management

Your PR fixes the problem with postgreSQL. Great job, thanks ! :+1:

I note that :

using "FOR UPDATE" slow down performances cause it makes concurrent transactions wait for each other. But doesn't seem significant cause I didn't notice any difference with my benchmark
When the ratelimit record doesn't exists yet in database, it's less accurate cause first concurrent transactions won't lock any row with "FOR UPDATE", and so, won't wait each other. So, you are able to call more than your ratelimit.

At all, this fix is OK for PostgreSQL ; I don't have any better suggestion to solve this problem.

My main concern now, is that it's not compatible with all DBMS supported by Gravitee. For example, it will totally break RateLimit with SQLServer, throwing this error : "FOR UPDATE clause allowed only for DECLARE CURSOR".

So we can't merge it like that, we have to adapt your work in order to solve this compatibility problem. I'll let you know about that.

marcambier commented 2 years ago

As there is currently no fix compatible with all DBMS ; We added a note in documentation, to discourage using JDBC as a rate limit repository, cause it can cause inaccuracies in the applied limit.

gravitee-io / issues