Dette passe 1, lot L4-bis : alertes même si l'écriture échoue, délai unique de 600 s, typage des capteurs, ordre des accusés MQTT #6
Loading…
Add table
Add a link
Reference in a new issue
No description provided.
Delete branch "pr/dette-l4bis"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
Objet
Lot L4-bis de la passe 1 de dette : corrections demandées par l'utilisateur à la relecture du lot L4 (et du lot L5 pour l'ordre des accusés). Quatre points : les alertes partent même si l'écriture de la mesure échoue, un délai unique de 600 s pour tous les types d'alerte, le typage des capteurs (valeur rattachée au capteur de sa famille), l'ordre des accusés MQTT exigé par MQTT 3.1.1.
Avant déploiement : appliquer la migration 003 (
scripts/db/apply-migration.sh, voir plus bas).Corrections et preuves
Insertedet aussi quand l'écriture de la mesure échoue (journal « mesure non enregistrée, alertes évaluées quand même ») ;Duplicaten'en lève aucune. Un échec passager est rejoué sans accusé, comme en L5. Lecture des capteurs en échec : nouvelle varianteDataError::Sensors, ni écriture ni alerte. Chaque alerte légionellose écrite seule : le délai ne démarre que pour les sondes dont l'alerte est en base.aquaserveur/src/data_handler.rs,database/store.rs(store_temp_alert),tests/data_alert_rule.rsALERT_COOLDOWN = 600 spour la légionellose,sw420et tout autre type ;LEGIONELLA_COOLDOWNethardware_cooldownsupprimés ;AlertManagerprend le même délai. Clés distinctes inchangées :(boîtier, sonde)et(boîtier, type).aquaserveur/src/alert_limits.rs,server.rssensors.rs: familleextouintdéduite du type (2,ds18b20,dsb18b20;1,dht11,dht22) ou lue dans la nouvelle colonnefamille; la Nième valeurextva au Nième capteurext; repli « par rang » si un capteur n'a pas de famille (pH, brome), avec avertissement. Seuils légionellose sur les seules sondes d'eau (ext, °C), numéro de sonde = colonnepositioncomme les seuils du legacy. Colonnefamille ENUM('ext','int') NULL INVISIBLEécrite au provisionnement.aquaserveur/src/sensors.rs,database/schema.rs,database/store.rs,provisioning/handler.rs,tests/sensor_typing.rsaquaserveur/src/ack_order.rs,server.rs,ingest.rs,worker.rs,tests/mqtt_server_integration.rsConséquence du point 3, à connaître : pour les 271 boîtiers legacy de la base, le rattachement « par rang » stockait la première sonde d'eau sous le capteur interne ; chaque valeur va désormais à son capteur, et les alertes 14 portent
capteur_id2 à 7 (au lieu de 1 à 6).Coût du point 4, à connaître : un message lent (verrou, rejeu) retient les accusés des messages reçus après lui ; au-delà de 20 messages non accusés (
max_inflight_messages), le broker attend.Tests et résultats
ALERT_COOLDOWNabsent,sensors11 échecs sur 11,ack_order6 échecs sur 7, ordre des accusés sur broker réel rouge sur le code L5.alert_limits(2),sensors(12),ack_order(7),data_handler,schema; 8 tests ignorés sur base et broker réels (data_alert_rule.rs,sensor_typing.rs,capteur_famille_migration.rs,mqtt_server_integration.rs).a_frame_of_two_thousand_values_alerts_only_for_known_sensorsattend une alerte au lieu de trois (la fixture 4000 n'a qu'une sonde d'eau ; c'est le défaut corrigé).cargo fmt --check,cargo clippy ... -D warnings: OK.cargo test --workspace: 350 passés, 0 échec, 76 ignorés (L5 : 332 et 68).log_type all: PUBACKMid 2(4000, écrit après) puisMid 3(4001, écrit avant).docker stoppendant un verrou de 6 s : arrêt en 4,3 s, 10 lignes, 10 PUBACK dans l'ordre, aucun renvoi au redémarrage.flume(déjà dans l'arbre parrumqttc).Migration et actions de déploiement
scripts/db/apply-migration.sh <conteneur> <fichier_env> scripts/db/migrations/003_capteur_famille.sql(droitsALTERetUPDATE, mot de passe parMYSQL_PWD). Idempotente : ajoutefamilleà chaque%_boitier_capteurs, remplit les lignes déductibles, ne modifie pas une famille déjà renseignée. Lire le bilan (tables_capteurs,colonnes_ajoutees,capteurs_types,capteurs_sans_famille,boitiers_rattaches_par_rang) ; d'après le dump,boitiers_rattaches_par_rangdevrait valoir 0 en production.type(erreur 1054 gérée) et se comporte de la même façon pour le legacy ; la migration reste nécessaire pour renseigner à la main les familles non déductibles (pH, brome).Choix faits
data_handler.rs.INVISIBLE, commemesure_empreinte(L4) : absente deSELECT *et desINSERTsans liste de colonnes de Laravel.INGEST_WORKERS=1: ordre strict sans perdre les écritures parallèles.Hors périmètre, pistes
fetch_legionella_thresholdslit encore une seule paire (LIMIT 1).AlertManager) : résultats non écrits, à revoir ou retirer.Retour arrière
Revenir les 3 commits et redéployer l'image précédente. La colonne
familleest invisible et nullable : l'ancien code et Laravel l'ignorent ; la retirer n'est pas nécessaire.Pile de PR
Base de cette PR :
pr/dette-l5. Fusion dans l'ordre de la pile ; apres chaque fusion,vigie.py retargetrebase la PR suivante surmain.pr/dette-l2: Dette passe 1, lot L2 : contrats partagés dans aquashared et client MQTT du boîtierpr/dette-l3: Dette passe 1, lot L3 : provisioning non destructif et identifiant validé à l'entrée MQTTpr/dette-l4: Dette passe 1, lot L4 : mesures, alertes et déduplicationpr/dette-l5: Dette passe 1, lot L5 : boucle MQTT du serveur, livraison de la configuration et journauxpr/dette-l4bis: Dette passe 1, lot L4-bis : alertes même si l'écriture échoue, délai unique de 600 s, typage des capteurs, ordre des accusés MQTT (cette PR)pr/dette-l7: Dette passe 1, lot L7 : builds reproductibles, dépendances, durcissement des conteneurspr/dette-l6: Dette passe 1, lot L6 : authentification Mosquitto, ACL par compte, healthchecks réels, pile e2e isoléepr/dette-l8: Dette passe 1, lot L8 : tests non destructifs, fixtures synthétiques, script de tests sur base jetableCommits du lot
000adbfrefactor: one 600 s alert delay for every alert kindd1b0be0feat: type frame values per sensor family and alert even when the write failsdfd85f7fix: send MQTT acknowledgements in reception orderRevue statique Vigie : PR #6
Tete
dfd85f78db, basepr/dette-l5(74df6f6fa5), 3 commit(s), 19 fichier(s) ajoutes ou modifies.Statut
aquaprocess/revue-statique: failure (semgrep (regles du depot))Constats : 1 bloquant, 1 majeur, 7 mineur, 0 style. Commentaires en ligne : 2 (mode
important, plafond 10).Verifications automatiques
Analyse qualitative
Verdict : les quatre corrections demandées sont faites et bien testées. Statut
failureà cause d'un seul point mécanique (règle semgrepsqlx-no-format-in-querysur une ligne de production, voir plus bas), facile à lever. 1 majeur de conception (retenue globale des accusés derrière un message en rejeu sans fin), le reste en mineurs.Plage revue :
74df6f6..dfd85f7(3 commits du lot), sans les lots précédents.Ce que cette PR corrige parmi les constats des PR #2 à #5
data_handler.rs:239: alertes court-circuitées par un échec d'écritureDuplicateseul les écarte (data_handler.rs:282à307)alert_limits.rs:41: trois sources de délaiALERT_COOLDOWNunique (alert_limits.rs:49), repris parserver.rsstore.rs:272: alertes légionellose hors transaction, délai non démarré pour les sondes écritesstore_temp_alert,store.rs:342)worker.rs:172:Duplicateaprès une écriture dont la réponse s'est perdue, alertes jamais partiesdata_handler.rs:34)Déjà relevé dans la pile, toujours présent (non repris en constat ici)
worker.rs:183(désormaisworker.rs:179, inchangé) : une trame refusée journaliseerror = %e, qui recopie un fragment du payload (Invalid ext sensor value: '...'). Ce lot ne le corrige pas ; il reste bloquant pour la pile.server.rs:282: un message refusé (file pleine) ou abandonné garde sa place inflight, aucune reconnexion n'est provoquée. Le séquenceur le reconduit tel quel (Release::Skip).worker.rs:99: rejeu sans plafond des erreurs passagères. Aggravé par ce lot, voir le majeurack_order.rs:30.worker.rs:121: accusé d'un message de l'ancienne connexion avec son ancien pkid ; le séquenceur garde ces tickets d'une connexion à l'autre, même risque.server.rs:177: la tâche de livraison garde un clone des files, le vidage d'arrêt attend souvent 8 s.data_handler.rs:234(désormais:275) : trame hors fenêtre d'epoch refusée sans alerte.config_ack_handler.rs:127: plafond par minute partagé entre l'alerte 15 et les alertes sanitaires.data_handler.rs:248(sw420perdu sur unDuplicate) etstore.rs:55(toute violation 1062 classée doublon).Contrôle automatique bloquant
aquaserveur/src/provisioning/handler.rs:456:sqlx::query(&format!("ALTER TABLE {table_capteurs} ADD COLUMN IF NOT EXISTS {FAMILLE_COLUMN_DDL}")). Les deux parties dynamiques sont sûres (nom de table dérivé d'unBoitierIdentier, constante), mais la règle du dépôt bloque toute requête construite dans l'appel. Correction minimale, celle du lot L7 pour les tests :let sql = format!(...); sqlx::query(&sql). Attention : la fusion de L7 (PR #7) ne la reprend pas, la ligne est identique à la tête de #7.Correction
DataError::Sensors(lecture des capteurs en échec) est bien distinguée de l'échec d'écriture et rejouée si la base est indisponible. Rejeu après échec passager : le délai par sonde évite la seconde alerte, prouvé sur base réelle para_successful_replay_after_a_transient_failure_raises_no_second_alert.every_alert_kind_waits_the_same_single_delayetan_alert_of_one_kind_does_not_hold_another_kindjustes.assign_typedest correct (Nièmeextau Nième capteurext,positioncomme numéro de sonde, surplus compté). Lecture tolérante à une table sansfamille(erreur 1054 seule, autre erreur rendue). Le repli « par rang » garde l'ancien défaut pour les sondes non température (voirsensors.rs:185).AckOrderest simple et juste ; ticket rendu une seule fois (consommé parack,Dropsinon) ; canal non borné justifié (borné en pratique parmax_inflight_messages). Arrêt : routeur, puis workers, puis séquenceur, puisDISCONNECT, dans cet ordre. Le coût (un message lent retient tous les accusés suivants) est documenté ; c'est sa combinaison avec le rejeu sans plafond qui le rend grave (ack_order.rs:30).003_capteur_famille.sql:75). ColonneINVISIBLEidentique au caractère près àFAMILLE_COLUMN_DDL, vérifiée par test.data_handler.rs:382, ligne déplacée, donc non commentée en ligne).Sécurité
Aucune donnée personnelle ajoutée aux journaux : les nouveaux journaux portent
boitier_id,probe,alert_list_id,valueet la nature de l'erreur. Les identifiants SQL dynamiques ajoutés viennent d'unBoitierIdvalidé ou de constantes. gitleaks : rien.Cohérence avec le compte rendu
Références vérifiées à la tête
dfd85f7, toutes exactes :alert_limits.rs:49,schema.rs:35,store.rs:186,:235,:342,data_handler.rs:260,:311,:344,sensors.rs:75,:106,:137,:177,ack_order.rs:63,:133,:174,server.rs:181,:224,ingest.rs:134,worker.rs:85,handler.rs:457. Résultats reproduits par Vigie :cargo test350 passés, 76 ignorés ; 76 tests ignorés passés sur base et brokers jetables, migration 003 appliquée par le dépôt.Écart : « le rejeu la tente de nouveau » ne vaut que pour une mesure non écrite (
data_handler.rs:34).Tests
TDD et contre-épreuves solides, y compris côté broker (
log_type all). Limites : l'attente du séquenceur avantDISCONNECTn'a aucun test qui la prouve (contre-épreuve non concluante, reconnue par le compte rendu) ; aucun test du repli « par rang » avec une sonde pH enext; aucun test d'un rejeu long qui retient les accusés des autres boîtiers.Questions ouvertes
Autres constats (non publies en ligne)
aquaserveur/src/ack_order.rs:26aquaserveur/src/ack_order.rs:271no-unwrap-in-production(WARNING) :.unwrap()ou.expect()peut paniquer en production. 10 occurrence(s) sur lignes ajoutees (271, 272, 296, 297, 298, 315, 326, 326, 339, 340), y compris d'eventuels modules de test internes.aquaserveur/src/ack_order.rs:337tokio-spawn-fire-and-forget(WARNING) :tokio::spawn()sans stocker le JoinHandle (fire-and-forget). 1 occurrence(s) sur lignes ajoutees (337), y compris d'eventuels modules de test internes.aquaserveur/src/data_handler.rs:34Insertedpuis alerte légionellose ou vibration en échec passager : l'erreur est journalisée, la trame est rendueProcessedet accusée, sans rejeu ; l'alerte n'est retentée qu'à la prochaine trame hors seuil (c'est la seconde moitié du constat #5 worker.rs:172, partiellement traitée). Acceptable si la condition persiste, mais à écrire tel quel dans l'en-tête, ou à traiter en rendantStep::Retryquand une alerte échoue en erreur passagère (le délai déjà démarré des autres sondes évite les doublons).aquaserveur/src/data_handler.rs:382admit_legionellaconsomme le plafond par minute avant l'écriture ; une alerte dont l'écriture échoue ne démarre pas de délai mais garde l'insertion consommée. Pendant le rejeu d'une écriture de mesure en échec (1 s doublé jusqu'à 30 s, sans plafond), chaque passage reconsomme le budget : il peut s'épuiser sans aucune alerte en base et refuser ensuite l'alerte 15 ou une autre sonde. Piste : ne consommer le budget qu'après une écriture réussie, ou le rendre en cas d'échec.aquaserveur/src/sensors.rs:185extsont évaluées en légionellose, sans contrôle d'unité : une sonde pH ou brome déclarée enext(cas cité dans le compte rendu,firstco-C.sh) produit des alertes 14 sur une valeur qui n'est pas une température, ce que l'en-tête du module (ligne 28) annonce justement exclu. Le numéro de sonde y est aussi le rang et nonposition: lecapteur_iddes alertes d'un même boîtier change le jour où un opérateur renseignefamille. Piste : en repli, ne garder que les valeurs dont le capteur de même rang est en °C, et numéroter parpositionquand elle existe.scripts/db/migrations/003_capteur_famille.sql:75EXECUTE IMMEDIATE(lignes 75, 85, 93), exécutés avec les droits ALTER et UPDATE. Défense simple :REPLACE(t, '', '``')ou refus parSIGNALd'un nom hors^[0-9A-Za-z_]+$`.Limites
Revue publiee en
COMMENT: l'auteur des PR et le relecteur sont le meme compte, Forgejo refuse APPROVE et REQUEST_CHANGES. Le blocage s'exprime par le statut de commitaquaprocess/revue-statique. Analyse statique et tests automatises seulement, sans fusion ni deploiement.@ -0,0 +27,4 @@//! Aucun numéro ne reste donc en attente pour toujours.//!//! ## Coût//! Un message lent (base en attente d'un verrou, rejeu d'une erreur[majeur] Le coût est documenté, mais combiné au rejeu sans plafond des erreurs passagères (
process_until_done, worker.rs:105, relevé en mineur sur #5 à worker.rs:99), il change d'échelle : un seul message en rejeu (verrou sur la table d'un boîtier, erreur 1205 répétée, ALTER Laravel,Protocolclassé passager) retient désormais les PUBACK de tous les boîtiers. Au-delà de 20 messages non accusés (max_inflight_messages), Mosquitto cesse d'envoyer tout QoS 1 au serveur : la réception entière s'arrête, sans fin, alors qu'en L5 seul le worker concerné était suspendu. Piste : borner la durée de rejeu d'un message (par exemple 60 s), puis abandonner son ticket (Skip) et provoquer une reconnexion pour que le broker le renvoie, avec un compteur et un journalerror; ou isoler un boîtier bloqué comme le propose la section 9 du compte rendu.Source : revue.
@ -448,2 +453,4 @@);sqlx::query(&create_capteurs).execute(pool).await?;// Table laissée par une reprise incomplète, antérieure à la colonnesqlx::query(&format!([bloquant] semgrep
sqlx-no-format-in-query(ERROR) : Construction SQL dynamique détectée avec sqlx.Source : semgrep.
View command line instructions
Checkout
From your project repository, check out a new branch and test the changes.Merge
Merge the changes and update on Forgejo.Warning: The "Autodetect manual merge" setting is not enabled for this repository, you will have to mark this pull request as manually merged afterwards.