benoitdm-oslandia / pg_featureserv

Apache License 2.0

1 stars 0 forks source link

feat(catalog): Etag cache implementation into catalog - [merged] #124

Closed benoitdm-oslandia closed 1 year ago

benoitdm-oslandia commented 2 years ago

In GitLab by @nrevelant on Oct 14, 2022, 16:24

_Merges feat/catalogcache -> develop

support for PGFS_CACHE environment variable
etag cache implemented with active and passive mode
etag exchanges between clients and pg_featureserv are now Base64 encoded
tests implemented for etag management

close #51

benoitdm-oslandia commented 2 years ago

avant l'import

benoitdm-oslandia commented 2 years ago

cette function devrait appeler DecodeStrongEtag puis générer le json après

benoitdm-oslandia commented 2 years ago

pareil que pour l'autre cache

benoitdm-oslandia commented 2 years ago

test non fonctionnel pour le moment ? il vaut mieux le supprimer

benoitdm-oslandia commented 2 years ago

ok top @nrevelant ! 2-3 babioles à corriger et on verra demain pour le rebase !

benoitdm-oslandia commented 2 years ago

In GitLab by @nrevelant on Oct 20, 2022, 09:35

Commented on internal/service/db_test/handler_db_etag_test.go line 108

Pour le test TestWeakEtagFromDifferentRepresentationsDb: Actuellement il est vrai que seul le format JSON est implémenté et complet actuellement. Du coup c'est un peu en prévision car cette vérification me parait importante du point de vue des tests. Je peux créer un ticket pour y penser et supprimer ce test en attendant ?
Pour le test TestEtagHeaderIfNonMatchAfterReplaceDb : En fait une fois le merge fait avec le travail d'Amaury (trigger), il sera opé. J'ai cru bon de le conserver. Au pire peut-être que c'est lui qui pourra ajouter ce test.

benoitdm-oslandia commented 2 years ago

resolved all threads

benoitdm-oslandia commented 2 years ago

approved this merge request

benoitdm-oslandia commented 2 years ago

In GitLab by @azarz on Oct 20, 2022, 15:38

Commented on internal/data/cache_active.go line 86

Non testé

benoitdm-oslandia commented 2 years ago

In GitLab by @azarz on Oct 20, 2022, 15:39

Commented on internal/data/cache_passive.go line 68

Non testé

benoitdm-oslandia commented 2 years ago

In GitLab by @azarz on Oct 20, 2022, 15:43

Commented on internal/service/db_test/handler_db_etag_test.go line 74

TestEtagHeaderIfNoneMatchDb

benoitdm-oslandia commented 2 years ago

In GitLab by @nrevelant on Oct 20, 2022, 16:27

Commented on internal/service/db_test/handler_db_etag_test.go line 108

changed this line in version 7 of the diff

benoitdm-oslandia commented 2 years ago

In GitLab by @nrevelant on Oct 20, 2022, 16:27

added 1 commit

ad1aecd8 - etag cache finalization + format handling enhancement

Compare with previous version

benoitdm-oslandia commented 2 years ago

In GitLab by @jmkerloch on Oct 20, 2022, 17:13

Commented on internal/conf/config.go line 214

Il est possible de se passer de la variable cacheSize.

Le log peut être effectué en utilisant directement Configuration.Cache.MapSize

benoitdm-oslandia commented 2 years ago

In GitLab by @jmkerloch on Oct 20, 2022, 17:13

Commented on internal/data/cache.go line 21

Comme il s'agit d'une interface il y a une convention pour nommer les interfaces en finissant par er : https://go.dev/doc/effective_go#interface-names

A voir si on met ca en place sur le projet.

benoitdm-oslandia commented 2 years ago

In GitLab by @jmkerloch on Oct 20, 2022, 17:13

Commented on internal/data/cache.go line 45

Ces méthodes sont complétement dupliquées entre CacheActive et CachePassive.

A voir comment on peut les mutualiser.

benoitdm-oslandia commented 2 years ago

In GitLab by @jmkerloch on Oct 20, 2022, 17:13

Commented on internal/data/catalog_db.go line 648

Suppression des commentaires non utiles.

benoitdm-oslandia commented 2 years ago

In GitLab by @jmkerloch on Oct 20, 2022, 17:13

Commented on internal/data/cache_passive.go line 32

La map n'est pas utilisée, il serait préférable de l'enlever.

benoitdm-oslandia commented 2 years ago

In GitLab by @jmkerloch on Oct 20, 2022, 17:13

Commented on internal/data/db_sql.go line 135

Ajout d'un commentaire sur l'utilisation de xmin et le fait qu'il soit propre à postgis et utilisé pour définir le weakEtag

benoitdm-oslandia commented 2 years ago

In GitLab by @jmkerloch on Oct 20, 2022, 17:13

Commented on internal/service/db_test/handler_db_delete_test.go line 16

il faut plus se rajouter et pas enlever l'auteur d'origine.

benoitdm-oslandia commented 2 years ago

In GitLab by @nrevelant on Oct 21, 2022, 08:58

added 14 commits

ad1aecd8...4b72ea92 - 7 commits from branch develop
09ad8da8 - feat(catalog): Etag cache implementation into catalog
bfb900cc - etag tests can now be launched sequentially
eedbabac - former go version 1.13 patch, copyright update
e2120183 - new cache parameters (PGFS_CACHE, PGFS_CACHESIZE) and enhancements (etag...
3d78b2fc - cache instantiation fix in catalog
5a8c4235 - importing default configuration into db and mock testing environments
143d3076 - etag cache finalization + format handling enhancement

Compare with previous version

benoitdm-oslandia commented 2 years ago

In GitLab by @nrevelant on Oct 21, 2022, 10:37

Commented on internal/data/cache.go line 45

En les sortant dans un fichier du type etag_tools.go à importer je suppose

benoitdm-oslandia commented 2 years ago

added 1 commit

031d9569 - feat(cache): Etag cache implementation into catalog

Compare with previous version

benoitdm-oslandia commented 2 years ago

done!

benoitdm-oslandia commented 2 years ago

done!

benoitdm-oslandia commented 2 years ago

done!

benoitdm-oslandia commented 2 years ago

done!

benoitdm-oslandia commented 2 years ago

removed

benoitdm-oslandia commented 2 years ago

removed!

benoitdm-oslandia commented 2 years ago

resolved all threads

benoitdm-oslandia commented 2 years ago

changed this line in version 10 of the diff

benoitdm-oslandia commented 2 years ago

changed this line in version 10 of the diff

benoitdm-oslandia commented 2 years ago

changed this line in version 10 of the diff

benoitdm-oslandia commented 2 years ago

changed this line in version 10 of the diff

benoitdm-oslandia commented 2 years ago

changed this line in version 10 of the diff

benoitdm-oslandia commented 2 years ago

changed this line in version 10 of the diff

benoitdm-oslandia commented 2 years ago

changed this line in version 10 of the diff

benoitdm-oslandia commented 2 years ago

changed this line in version 10 of the diff

benoitdm-oslandia commented 2 years ago

changed this line in version 10 of the diff

benoitdm-oslandia commented 2 years ago

changed this line in version 10 of the diff

benoitdm-oslandia commented 2 years ago

changed this line in version 10 of the diff

benoitdm-oslandia commented 2 years ago

changed this line in version 10 of the diff

benoitdm-oslandia commented 2 years ago

changed this line in version 10 of the diff

benoitdm-oslandia commented 2 years ago

added 1 commit

3cd46459 - feat(cache): Etag cache implementation into catalog

Compare with previous version

benoitdm-oslandia commented 2 years ago

@nrevelant j'ai squashé et fais le ménage. J'ai aussi créé le fichier etag.go dans api. Désolé, je sais que ce n'est pas très sympa mais comme ca on peut avancer.

benoitdm-oslandia commented 2 years ago

requested review from @benoitdm-oslandia

benoitdm-oslandia commented 2 years ago

In GitLab by @nrevelant on Oct 21, 2022, 17:46

En fait la question est plus au niveau des délais et le niveau de satisfaction quant à ces derniers. Je n'ai rien ajouté comme code aujourd'hui car j'attendais que tu finisses le merge que tu souhaitais faire. Du coup je suis passé sur autre chose cette après-midi. De plus j'ai été plusieurs fois en attente ces derniers jours lorsque j'ai eu besoin d'aide ou que tu tranches un sujet.

Je sais que tu es occupé mais n'oublie pas que nous sommes à distance, et pas dans la même boîte. Je ne sais pas forcément ce sur quoi tu travailles à un instant donné, et ne suis physiquement pas à côté de toi en cas de besoin d'un côté ou de l'autre. N'hésite donc pas à me signaler quand tu es disponible ou pas.

benoitdm-oslandia commented 2 years ago

In GitLab by @lowzonenose on Oct 21, 2022, 18:36

:warning: BUG

“Maps are not safe for concurrent use”.

Méthode pour reproduire le BUG

1er test simple (1 requête)

export PGFS_CACHE=0
./pg_featureserv --debug

$ curl -H Accept:json -H Content-Type:application/json -H If-None-Match:"InB1YmxpYy5tb2NrX2EtNDMyNi1qc29uLTU3OTUxIg==" http://localhost:9000/collections/public.mock_a/items/1

:ok: j'ai une réponse

2eme test en mode test de charge en concurrence (10 goroutines)

Sans cache

export PGFS_CACHE=0
./pg_featureserv --debug

$ GOPATH/bin/go-wrk -H Accept:json -H Content-Type:application/json -H If-None-Match:"InB1YmxpYy5tb2NrX2EtNDMyNi1qc29uLTU3OTUxIg==" http://localhost:9000/collections/public.mock_a/items/1

:ok: j'ai une réponse

Running 10s test @ http://localhost:9000/collections/public.mock_a/items/1
  10 goroutine(s) running concurrently
49651 requests in 9.963772112s, 15.20MB read
Requests/sec:       4983.15
Transfer/sec:       1.53MB
Avg Req Time:       2.006761ms
Fastest Request:    330.33µs
Slowest Request:    19.198466ms
Number of Errors:   0

3eme test en mode test de charge en concurrence (10 goroutines)

Avec Cache

export PGFS_CACHE=1
export PGFS_CACHESIZE=4000
./pg_featureserv --debug

$ GOPATH/bin/go-wrk -H Accept:json -H Content-Type:application/json -H If-None-Match:"InB1YmxpYy5tb2NrX2EtNDMyNi1qc29uLTU3OTUxIg==" http://localhost:9000/collections/public.mock_a/items/1

Running 10s test @ http://localhost:9000/collections/public.mock_a/items/1
  10 goroutine(s) running concurrently
120 requests in 38.930685ms, 37.62KB read
Requests/sec:       3082.40
Transfer/sec:       966.26KB
Avg Req Time:       3.244223ms
Fastest Request:    598.938µs
Slowest Request:    26.273783ms
Number of Errors:   263678

:no_entry: exception de pg_featureserv:

fatal error: concurrent map writes

4eme test en mode test de charge sans concurrence

Avec Cache

export PGFS_CACHE=1
export PGFS_CACHESIZE=40000
./pg_featureserv --debug

$GOPATH/bin/go-wrk -c 1 -H Accept:json -H Content-Type:application/json -H If-None-Match:"InB1YmxpYy5tb2NrX2EtNDMyNi1qc29uLTU3OTUxIg==" http://localhost:9000/collections/public.mock_a/items/1
Running 10s test @ http://localhost:9000/collections/public.mock_a/items/1
  1 goroutine(s) running concurrently
16754 requests in 9.883038883s, 5.13MB read
Requests/sec:       1695.23
Transfer/sec:       531.41KB
Avg Req Time:       589.891µs
Fastest Request:    408.609µs
Slowest Request:    16.403973ms
Number of Errors:   0

:ok: ça fonctionne !

Pistes de résolution

:eye_in_speech_bubble: cf. https://ashish.one/blogs/fatal-error-concurrent-map-writes/

:eye_in_speech_bubble: cf. https://dev.to/sreramk/go-loadanddelete-and-loadorstore-in-sync-map-why-are-they-needed-30f7

:pushpin: https://go.dev/tour/concurrency/9

:eyes: [performance] https://medium.com/@deckarep/the-new-kid-in-town-gos-sync-map-de24a6bf7c2c

benoitdm-oslandia commented 2 years ago

In GitLab by @nrevelant on Oct 21, 2022, 19:53

Ok il faut une map adaptée du genre sync.Map du coup

benoitdm-oslandia commented 2 years ago

In GitLab by @lowzonenose on Oct 21, 2022, 22:18

oui

Previous Next