search

etalab / transport-site

Rendre disponible, valoriser et améliorer les données transports
https://transport.data.gouv.fr
194 stars 30 forks source link

"RangeError: Out of memory" sur un appel API via Swagger #2083

Closed thbar closed 2 years ago

thbar commented 2 years ago

Si je vais ici:

https://transport.data.gouv.fr/swaggerui#/datasets/API.DatasetController.datasets

et que je clique sur "execute", j'obtiens :

CleanShot 2022-01-31 at 10 05 10@2x

Le site n'a pas l'air d'être tombé pour autant.

thbar commented 2 years ago

Si je fais une query, je vois que le retour d'API fait 17 mégas. Certaines ressources prennent beaucoup de place en terme de JSON, et ça semble lié à l'output des validations qui est intégré directement. Je vais essayer de regrouper quelques stats.

thbar commented 2 years ago

Plutôt que jq, j'ai utilisé un script Elixir et c'était drôlement plus pratique pour filtrer ça:

Mix.install([:jason])

"data.json"
|> File.read!()
|> Jason.decode!()
|> Enum.map(&(&1["resources"]))
|> List.flatten
|> Enum.map(fn(x) -> {x["datagouv_id"], x |> get_in(["metadata", "validation", "errors_count"]) } end)
|> Enum.filter(fn({_d, c}) -> c != nil && c > 10 end)
|> IO.inspect(IEx.inspect_opts)

Le retour est le suivant:

[
  {"65988d54-1ba2-4ea5-86c8-2ba30162e211", 97441},
  {"aba5743e-230f-49dd-8270-39efc5a72460", 6663},
  {"8218f747-8c53-49d3-8470-24c0afef3dfb", 13594},
  {"50625621-18bd-43cb-8fde-6b8c24bdabb3", 1000},
  {"d732ee90-83df-4161-af71-9aa2cf23952e", 1576},
  {"b11dffe1-dcf9-438e-99bb-23cca0a6761a", 20056},
  {"9ca17d67-3ba3-410b-9e32-6ac7948c3e06", 27660},
  {"ffbd75ac-8e9b-45c2-984d-97599d33d00d", 4441},
  {"da69c1d8-68ff-4068-8e5a-b9bd201f8988", 32},
  {"1da34e46-dca1-4ace-be79-41c1116c9530", 40},
  {"cb26e169-b319-42fd-8d98-be61b94c27a6", 1074},
  {"8ed8a6e7-0385-4a87-b0cc-15e4a24fcd26", 28},
  {"4eb730eb-34e3-4456-a42c-75b36e7a6a55", 13},
  {"a6a84a04-5c1d-4a5b-aa5d-a0734cdac810", 17336}
]
thbar commented 2 years ago

En googlant un peu, je vois que RangeError: Out of memory est un message typiquement Javascript. Je pense que la payload est peut-être digérée côté client (si j'ai compris comment cette implémentation Swagger fonctionne), et que la taille pourrait bien être le point bloquant.

thbar commented 2 years ago

Ca pourrait également expliquer le souci d'erreur 500 côté Zapier dans le test réalisé par @NicolasBerthelot (17M c'est vraiment inhabituel pour une payload JSON d'API simple).

thbar commented 2 years ago

En affinant je vois que ces grosses payloads sont toutes liées à de la validation jsonschema:

Mix.install([:jason])

"data.json"
|> File.read!()
|> Jason.decode!()
|> Enum.map(&(&1["resources"]))
|> List.flatten
|> Enum.filter(fn(d) ->
  c = get_in(d, ["metadata", "validation", "errors_count"])
  c != nil && c > 10
end)
|> Enum.map(&(&1["metadata"]["validation"]["schema_type"]))
|> IO.inspect(IEx.inspect_opts)
["jsonschema", "jsonschema", "jsonschema", "tableschema", "jsonschema",
 "jsonschema", "jsonschema", "jsonschema", "jsonschema", "jsonschema",
 "jsonschema", "jsonschema", nil, "jsonschema"]

https://github.com/etalab/transport-site/issues/1850 aurait permis d'avoir un peu plus de précisions, donc un nouveau vote pour implémenter cet ajout de champs dans l'API.

Je crée un ticket séparé et clair pour indiquer qu'il faudra "trimmer" la payload par défaut.

AntoineAugusti commented 2 years ago

Fixed by #2097

AntoineAugusti commented 2 years ago

Je vous laisserai fermer cette issue une fois que vous aurez vérifié qu'il n'y a pas de problème à appeler l'API via un logiciel tiers/Swagger UI

thbar commented 2 years ago

Ca a bien fondu (5,5MB au lieu de 17MB) mais Swagger plante encore. Je vais partager un petit "breakdown" ici, je garde assigné.

AntoineAugusti commented 2 years ago

Je suis à 3.1MB avec l'onglet "Network" de Chrome

thbar commented 2 years ago

J'ai pareil à présent en fait ; j'ai peut-être pretty printé au passage.

thbar commented 2 years ago

@AntoineAugusti ah non je pense que c'est peut-être que j'ai lancé la requête après le deploy, mais avant ton nettoyage ? Enfin bref. Je vais quand même analyser car ça reste trop gros pour Swagger.

thbar commented 2 years ago

Les choses que j'ai pu vérifier jusque là sont les suivantes:

Analyse de la structure

J'ai lancé une première analyse avec https://www.debugbear.com/json-size-analyzer sur l'output de https://transport.data.gouv.fr/api/datasets, et j'ai expandé un peu le résultat, ça donne ceci:

CleanShot 2022-02-07 at 15 39 39@2x

Dette technique associée à Swagger

Voir https://github.com/open-api-spex/open_api_spex/issues/421, on est sur une version EOL (depuis peu) servie par un plug. J'ai regardé car j'essayais de comprendre si en mettant à jour, et avec le fix #2097, ça aurait pu suffire.

Utilisation d'un SwaggerUI plus récent en local

Avec la commande suivante:

docker pull swaggerapi/swagger-ui 
docker run -p 80:8080 swaggerapi/swagger-ui

On peut ensuite saisir l'url de notre JSON Swagger dispo ici https://transport.data.gouv.fr/api/openapi.

J'obtiens les résultats suivants:

Conclusions pour le moment

Notre API est trop verbeuse. Il faudra:

thbar commented 2 years ago

Le test sous Zapier est toujours non concluant ; je pense que 3 MB ça reste bien trop gros pour un retour d'API JSON "classique", il va falloir voir si on fait fondre davantage, quitte à ensuite permettre d'autres opérations.

thbar commented 2 years ago

J'ai une bonne piste en fait : on a 2 datasets qui sont des "baleines". Je vais partager mes analyses, mais ça devrait faire fondre pas mal le tout.

thbar commented 2 years ago

Après un peu de scripting, j'ai effectivement la preuve qu'on a 2 datasets qui sont très imposants, et c'est lié aux ressources communautaires:

Voilà l'analyse: j'ai utilisé l'API pour déterminer quels sont les datasets dont la partie d'API, ré-encodée en JSON, est au dessus de 20k:

[
  %{
    id: "5eab66a565c27001ac5f11ed",
    page_url: "https://transport.data.gouv.fr/datasets/zfe-zone-a-faibles-emissions",
    reencoded_byte_size: 21693
  },
  %{
    id: "5b4f2f52c751df4ea419aef3",
    page_url: "https://transport.data.gouv.fr/datasets/base-de-donnees-multimodale-transports-publics-en-bretagne-mobibreizh",
    reencoded_byte_size: 23720
  },
  %{
    id: "5f3c6c44cadb68fa4c58be26",
    page_url: "https://transport.data.gouv.fr/datasets/arrets-horaires-et-parcours-theoriques-gtfs-des-differents-reseaux-de-transport-membres-du-synd3-1",
    reencoded_byte_size: 988827
  },
  %{
    id: "5b3b3de988ee38708ada1789",
    page_url: "https://transport.data.gouv.fr/datasets/reseaux-de-transports-en-commun-de-la-metropole-daix-marseille-provence-et-des-bouches-du-rhone",
    reencoded_byte_size: 714734
  }
]

Le script est le suivant:

Mix.install([
  :jason, :req
  ])

defmodule FileCache do
  def fetch!(filename, cb) do
    unless File.exists?(filename), do: File.write!(filename, cb.())
    File.read!(filename)
  end
end

datasets = FileCache.fetch!("data.json", fn ->
  %{status: 200, body: body} = Req.build(:get, "https://transport.data.gouv.fr/api/datasets") |> Req.run!()
  body
end)

datasets
|> Jason.decode!()
|> Enum.map(fn(dataset) ->
  %{
    id: dataset["id"],
#    dataset: dataset,
    page_url: dataset["page_url"],
    reencoded_byte_size: dataset |> Jason.encode!() |> byte_size()
  }
end)
|> Enum.filter(fn(d) -> d[:reencoded_byte_size] > 20_000 end)
|> Enum.map(fn(dataset) ->
  dataset
end)
|> IO.inspect(IEx.inspect_opts)
thbar commented 2 years ago

Il semblerait que #2098 n'aie pas suffi à nettoyer pour les deux gros datasets, car j'ai le même résultat:

[
  %{
    id: "5eab66a565c27001ac5f11ed",
    page_url: "https://transport.data.gouv.fr/datasets/zfe-zone-a-faibles-emissions",
    reencoded_byte_size: 21693
  },
  %{
    id: "5f3c6c44cadb68fa4c58be26",
    page_url: "https://transport.data.gouv.fr/datasets/arrets-horaires-et-parcours-theoriques-gtfs-des-differents-reseaux-de-transport-membres-du-synd3-1",
    reencoded_byte_size: 988827
  },
  %{
    id: "5b3b3de988ee38708ada1789",
    page_url: "https://transport.data.gouv.fr/datasets/reseaux-de-transports-en-commun-de-la-metropole-daix-marseille-provence-et-des-bouches-du-rhone",
    reencoded_byte_size: 714734
  }
]

Il va falloir enquêter.

thbar commented 2 years ago

Je me suis appuyé sur l'API qui retourne la donnée d'un dataset donné, pour afficher cela plus facilement:

Si je pipe ça dans jq, j'ai:

❯ curl --silent https://transport.data.gouv.fr/api/datasets/5f3c6c44cadb68fa4c58be26 | jq ".community_resources | length" 
1418

❯ curl --silent https://transport.data.gouv.fr/api/datasets/5b3b3de988ee38708ada1789 | jq ".community_resources | length"
938

Donc sauf erreur on a encore 1418+938 ressources communautaires à purger ici.

fchabouis commented 2 years ago

Si je vais sur la page data.gouv.fr du premier : https://www.data.gouv.fr/fr/datasets/arrets-horaires-et-parcours-theoriques-gtfs-des-differents-reseaux-de-transport-membres-du-synd3-1/#community-resources Il en reste 13.

Et le 2ème, il en reste 0. https://www.data.gouv.fr/fr/datasets/reseaux-de-transports-en-commun-de-la-metropole-daix-marseille-provence-et-des-bouches-du-rhone/#community-resources

C'est plus un problème lors de l'import data.gouv => transport visiblement.

fchabouis commented 2 years ago

Effectivement, j'ai essayé d'importer à la main, et ça échoue, probablement à cause de la taille de la transaction. Je vais supprimer les ressources communautaires à la main.

le dataset 146 : image

et le 506 : image

thbar commented 2 years ago

Super merci @fchabouis d'avoir été voir de l'autre côté !

thbar commented 2 years ago

Les payloads sont bien allégées de leurs ressources communautaires :

❯ curl --silent https://transport.data.gouv.fr/api/datasets/5f3c6c44cadb68fa4c58be26 | jq ".community_resources | length" 
13

❯ curl --silent https://transport.data.gouv.fr/api/datasets/5b3b3de988ee38708ada1789 | jq ".community_resources | length"
0

Par contre il reste un souci de taille malgré tout, je regarde de plus près.

thbar commented 2 years ago

Les datasets ont fondu, les plus gros sont à présent:


  %{
    id: "5eab66a565c27001ac5f11ed",
    page_url: "https://transport.data.gouv.fr/datasets/zfe-zone-a-faibles-emissions",
    reencoded_byte_size: 21693
  },
  %{
    id: "5f3c6c44cadb68fa4c58be26",
    page_url: "https://transport.data.gouv.fr/datasets/arrets-horaires-et-parcours-theoriques-gtfs-des-differents-reseaux-de-transport-membres-du-synd3-1",
    reencoded_byte_size: 22778
  }
]
thbar commented 2 years ago

On est passé de 3.1 MB à 900k à présent (en partant de 17 MB à l'origine):

$ curl -I https://transport.data.gouv.fr/api/datasets | grep content-length
content-length: 923898

Safari met super longtemps (plusieurs minutes) à digérer l'appel Swagger pour https://transport.data.gouv.fr/swaggerui#/datasets/API.DatasetController.datasets, mais à présent ça fonctionne.

Idem de l'autre côté avec Chrome.

Je vais avoir avec @NicolasBerthelot si ça suffit pour avancer dans Zapier, puis je clôturerai ici.

Ma conclusion est quand même qu'il faudrait alléger le retour principal de l'API, avec moins de choses dedans par défaut, et probablement une forme de pagination (même si pour les outils comme Zapier après ça complique un peu la donne), si on ne peut pas alléger, car en l'état pas mal d'outils vont avoir un peu de mal !

thbar commented 2 years ago

J'ai créé un ticket spécifique (#2101) pour le problème de Zapier, dont @NicolasBerthelot a confirmé qu'il continuait à se produire malgré les optimisations (mais qui ne sont pas perdues, elles étaient nécessaires je pense).