feat(catalog_db): add trigger and listener to database events - [merged]

[benoitdm-oslandia]

In GitLab by @azarz on Oct 10, 2022, 16:00

Merges feature/listen-notify -> develop

close #50

[benoitdm-oslandia]

In GitLab by @azarz on Oct 17, 2022, 10:20

added 1 commit

• b6966154 - feat(catalog_db): listen to channel in goroutine

Compare with previous version

[benoitdm-oslandia]

oui listener et trigger car ils sont tous liés, listener.go c'est pas mal !

[benoitdm-oslandia]

In GitLab by @azarz on Oct 17, 2022, 16:59

Commented on internal/data/catalog_db.go line 609

changed this line in version 11 of the diff

[benoitdm-oslandia]

In GitLab by @azarz on Oct 17, 2022, 16:59

Commented on internal/data/catalog_db.go line 220

changed this line in version 11 of the diff

[benoitdm-oslandia]

In GitLab by @azarz on Oct 17, 2022, 16:59

Commented on internal/data/catalog_db.go line 132

changed this line in version 11 of the diff

[benoitdm-oslandia]

In GitLab by @azarz on Oct 17, 2022, 16:59

Commented on internal/data/catalog_db.go line 53

changed this line in version 11 of the diff

[benoitdm-oslandia]

In GitLab by @azarz on Oct 17, 2022, 16:59

added 1 commit

• 1f4211e0 - DRAFT: feat(listener): move listener methods to listener.go, rename...

Compare with previous version

[benoitdm-oslandia]

In GitLab by @azarz on Oct 17, 2022, 17:01

J'ai bougé les fonctions dans un nouveau fichier, mais j'ai l'impression que le listener ne marche plus (en tout cas dans les tests : j'ai fait un test d'intégration "à la main" et ça semblait marcher.)

[benoitdm-oslandia]

bizarre parce que ca a l'air de fonctionner avec les types interface : cf. https://www.golinuxcloud.com/golang-mutex/#Race_condition_example

[benoitdm-oslandia]

In GitLab by @azarz on Oct 17, 2022, 17:57

Commented on internal/data/listener.go line 148

Temporaire le temps que je fasse mes tests

[benoitdm-oslandia]

pour le coup tu vas devoir mettre plein de log ...

[benoitdm-oslandia]

In GitLab by @azarz on Oct 18, 2022, 11:28

added 1 commit

• a5d782ad - DRAFT: feat(listener): move listener methods to listener.go, rename...

Compare with previous version

[benoitdm-oslandia]

In GitLab by @azarz on Oct 18, 2022, 11:30

Commented on internal/service/db_test/handler_db_listener_test.go line 29

@lowzonenose Ce test ne marche pas, alors que...

[benoitdm-oslandia]

In GitLab by @azarz on Oct 18, 2022, 11:30

Commented on internal/service/db_test/handler_db_listener_test.go line 65

@lowzonenose ... celui-ci passe... Je ne comprends pas trop pourquoi, j'ai l'impression que c'est à cause de l'accès concurrent au cache quand le runner fait tout tourner en parallèle ?

[benoitdm-oslandia]

Aucun des 2 ne passent qd je les exécute en local

[benoitdm-oslandia]

On est bien d'accord que tu ne mets jamais à jour le cache ? :innocent:

[benoitdm-oslandia]

In GitLab by @azarz on Oct 18, 2022, 14:38

Commented on internal/service/db_test/handler_db_listener_test.go line 65

Je viens de vérifier chez moi (go version go1.19 linux/amd64), le test que je dis passer passe :/

go test -v -run TestCacheSizeIncreaseAfterCreateNoRunner github.com/CrunchyData/pg_featureserv/internal/service/db_test

time="2022-10-18T14:34:18+02:00" level=warning msg="No env var 'DATABASE_URL' defined, using default value: postgresql://postgres@localhost/pg_featureserv"
time="2022-10-18T14:34:18+02:00" level=info msg="Connected as postgres to pg_featureserv @ localhost"
=== RUN   TestCacheSizeIncreaseAfterCreateNoRunner
map[4044:map[geometry:map[coordinates:[12 34] crs:map[properties:map[name:EPSG:4326] type:name] type:Point] id:28 prop_a:propA prop_b:99 prop_c:propC prop_d:9]]
--- PASS: TestCacheSizeIncreaseAfterCreateNoRunner (0.12s)
PASS
ok      github.com/CrunchyData/pg_featureserv/internal/service/db_test  0.618s

Le cache est censé être mis à jour dans le listener dès qu'il y a un INSERT, du coup le POST fait "manuellement" est censé déclencher une maj du cache (avec un beau Println en dur dans le code pour vérifier, d'ailleur c'est ce qui s'affiche dans mon output).

[benoitdm-oslandia]

In GitLab by @azarz on Oct 18, 2022, 15:29

added 1 commit

• df475e31 - DRAFT: feat(listener): move listener methods to listener.go, rename...

Compare with previous version

[benoitdm-oslandia]

In GitLab by @azarz on Oct 18, 2022, 17:16

Commented on internal/data/listener.go line 148

changed this line in version 14 of the diff

[benoitdm-oslandia]

In GitLab by @azarz on Oct 18, 2022, 17:16

Commented on internal/service/db_test/handler_db_listener_test.go line 65

changed this line in version 14 of the diff

[benoitdm-oslandia]

In GitLab by @azarz on Oct 18, 2022, 17:16

added 1 commit

• 06661d0c - feat(listener): move listener methods to listener.go, rename...

Compare with previous version

[benoitdm-oslandia]

In GitLab by @azarz on Oct 18, 2022, 17:23

Les tests ne passaient pas dans le runner, parce que dans le beforeEachRun on fait un DROP TABLE CASCADE qui vire les triggers. Du coup rien n'est écouté.

Pour l'instant j'ai réglé le souci en déplaçant l'initialisation du cache, mais je n'aime pas cette solution. Enlever les DROP TABLE fait qu'à chaque run, les milliers d'INSERT sont écoutés par le cache, et mon ordinateur n'appécie pas trop (ça finit par marcher mais au bout de 60 seondes contre 4 secondes sinon).

[benoitdm-oslandia]

au dessus des imports stp

[benoitdm-oslandia]

func (listener *listenerDB) addNotifiyFunctionToDB() {

[benoitdm-oslandia]

pareil, avant les imports stp

[benoitdm-oslandia]

toujours utile ?

[benoitdm-oslandia]

In GitLab by @azarz on Oct 19, 2022, 09:45

Commented on internal/data/listener.go line 173

J'ai repris la nomenclature postgres : https://www.postgresql.org/docs/current/plpgsql-trigger.html

[benoitdm-oslandia]

In GitLab by @azarz on Oct 19, 2022, 09:46

Commented on internal/data/listener.go line 5

Mettre à jour https://git.oslandia.net/Client-projects/geoplateforme-ign-pg-featureserv/-/issues/52 alors ;)

[benoitdm-oslandia]

In GitLab by @azarz on Oct 19, 2022, 09:48

Commented on internal/service/db_test/runner_db_test.go line 161

Oui je pense, car dans le cat.Close il y a la fermeture de la connexion à la bdd, et la fermeture du listener qui droppe les triggers et schema temporaire :

listener.dropTriggers()
listener.dropTemporaryDBSchema()

[benoitdm-oslandia]

ok

[benoitdm-oslandia]

en effet !

[benoitdm-oslandia]

In GitLab by @azarz on Oct 19, 2022, 10:12

added 1 commit

• 13c85969 - feat(listener): move listener methods to listener.go, rename...

Compare with previous version

[benoitdm-oslandia]

In GitLab by @azarz on Oct 19, 2022, 10:15

Commented on internal/data/listener.go line 173

Pour les personnes qui connaissent postgresql, je pense donc que triggerFunction leur parlera plus. J'ai peut-être tort !

[benoitdm-oslandia]

bon ok ... ca ne change pas gd chose fondamentalement

[benoitdm-oslandia]

In GitLab by @azarz on Oct 20, 2022, 16:42

Commented on internal/service/db_test/handler_db_listener_test.go line 29

changed this line in version 16 of the diff

[benoitdm-oslandia]

In GitLab by @azarz on Oct 20, 2022, 16:42

added 1 commit

• 1ef3035b - feat(listener): move listener methods to listener.go, rename...

Compare with previous version

[benoitdm-oslandia]

In GitLab by @azarz on Nov 3, 2022, 18:40

added 10 commits

• 1ef3035b...acfab526 - 6 commits from branch develop
• da874d6f - feat(catalog_db): Add triggers to included tables after insert or update or delete
• 7d5346e1 - feat(catalog_db): listen to channel in goroutine
• 3013f4ae - feat(listener): move listener methods to listener.go, rename...
• 2774dd3f - feat(cache listener): adapt listener to Cacher and add RemoveWeakEtag() to Cacher

Compare with previous version

[benoitdm-oslandia]

In GitLab by @azarz on Nov 3, 2022, 18:43

marked this merge request as ready

[benoitdm-oslandia]

In GitLab by @azarz on Nov 3, 2022, 18:44

resolved all threads

[benoitdm-oslandia]

added 2 commits

• efd3f8dc - db: remove addLeadingComma parameter to function sqlColList to improve sql readability
• 7a1f49fe - feat(cache listener): add String and Size to Cacher and fix some log

Compare with previous version

[benoitdm-oslandia]

added 3 commits

• 4eae528f - feat(listener): add db listener
• 159e5a86 - feat(cache listener): adapt listener to Cacher
• 90fdbb2a - feat(db): remove addLeadingComma parameter to function sqlColList to improve sql readability

Compare with previous version

[benoitdm-oslandia]

requested review from @benoitdm-oslandia

[benoitdm-oslandia]

In GitLab by @azarz on Nov 4, 2022, 14:13

Commented on internal/service/db_test/handler_db_listener_test.go line 167

Bizarre que le test passe non ? Maintenant avec la nouvelle méthode cache.Size(), le nombre d'entrées dans le cache ne devrait pas changer avec un update, si ?

[benoitdm-oslandia]

In GitLab by @azarz on Nov 4, 2022, 14:13

Commented on internal/service/db_test/handler_db_listener_test.go line 193

Bizarre que le test passe non ? Maintenant avec la nouvelle méthode cache.Size(), le nombre d'entrées dans le cache ne devrait pas changer avec un update, si ?

[benoitdm-oslandia]

approved this merge request

[benoitdm-oslandia]

In GitLab by @nrevelant on Nov 4, 2022, 15:20

Commented on internal/service/db_test/handler_db_listener_test.go line 38

Super test. Par contre peut-être que l'on pourrait avoir un compteur des entrées dans le cache incrémenté/décrémenté à chaque ajout/suppression d'entrée ? Je pense que ce type d'opération n'est pas coûteux en temps machine. Et ça permettrait de mieux gérer notamment le fait de préallouer les blocs mémoire pour la map (ça me semble plus coûteux des I/O en temps machine). Qu'st-ce que vous en dites ? @azarz @benoitdm-oslandia ?

[benoitdm-oslandia]

In GitLab by @nrevelant on Nov 4, 2022, 15:30

Commented on internal/data/listener.go line 69

Il semble que l'on va travailler sur une seconde copie du cache dans le listener (en plus de celle du catalogue qui en contient déjà un en attribut). Peut-être que l'on pourrait utiliser un pointeur sur le cache pour éviter de dupliquer ?

[benoitdm-oslandia]

In GitLab by @nrevelant on Nov 4, 2022, 15:32

Commented on internal/data/listener.go line 4

2022 ?

[benoitdm-oslandia]

In GitLab by @azarz on Nov 4, 2022, 15:34

Commented on internal/service/db_test/handler_db_listener_test.go line 38

Tu es sur une vieille version du diff : @benoitdm-oslandia a ajouté la méthode Cacher.Size() qui permet d'obtenir la taille du cache. Après, peut-être qu'elle ne marche pas comme prévu et renvoie toujours la taille préallouée.

[benoitdm-oslandia]

In GitLab by @azarz on Nov 4, 2022, 15:39

Commented on internal/data/listener.go line 69

Effectivement, avant mon code utilisait une map qui en go est toujours passée par référence (https://stackoverflow.com/questions/40680981/are-maps-passed-by-value-or-by-reference-in-go).

Mais avec une variable du type Cacher, il y a moyen que ce soit passé par valeur...