Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Rework notices database table, split out skymaps #85

Open
martinjohndyer opened this issue Jul 23, 2024 · 0 comments
Open

Rework notices database table, split out skymaps #85

martinjohndyer opened this issue Jul 23, 2024 · 0 comments
Labels
-database Issues/PRs related to inserting into the database

Comments

@martinjohndyer
Copy link
Member

martinjohndyer commented Jul 23, 2024

In practice, there are a few weaknesses of the database structure added in #71 that I think should be corrected:

  • The reliance on IVORNs as unique keys makes moving away from VOEvents tricky (as added in Add new GCN alerts and SCIMMA broker #84). Having a unique key for alerts was never particularly necessary, just a nice sanity check when testing that the same alert wasn't being added multiple times. However Add new GCN alerts and SCIMMA broker #84 adds backdating to the sentinel through Kafka, which required a function to check if a given notice had already been processed, which uses the IVORN. It's not a real problem as long as we make a fake IVORN for the new style notices, which I've added for the EP alerts (Add Einstein Probe alerts #80), and will work once Add new GCN alerts and SCIMMA broker #84 is merged. But for the new IGWN alerts we can't construct the fake IVORNs in a way that mirrors the old alerts, and that prevents us using both as a redundancy system (see Process new-style GCN and IGWN alerts #82 (comment)). I think the correct thing to do would be to scrap IVORNs and have a standard combination of notice source and timestamp which should be consistent to prevent any duplicate notices (I did wonder about a full comparison of the payload, but it seems IGWN AVRO and JSON alerts can have the same keys in different orders, and I don't want to mess with that).
  • We should specify that the payload should be saved in JSON format, to make processing simpler. XML and AVRO alerts are already converted internally, we shouldn't save e.g. the AVRO blob when we could save the JSON and make it easier to process. Also see the comment Process new-style GCN and IGWN alerts #82 (comment) about removing the embedded skymaps before saving IGWN alerts.
  • I also think we should have a separate Skymaps table to save the Notices table filling up quite so quickly. Saving the skymaps in the database takes a lot of space, and when I'm testing I'd like to be able to copy the notices table without all the skymap information. In an ideal world we'd also check to see if the skymap is unchanged when new notices are added, which would cut down on duplicating data.
  • Oh and change the Event.type parameter since it's a built-in (see the control.exposures table).

None of these are included in #84 because they're not strictly necessary, and since the GOTO Marshall reads the alert database I don't want to make any changes without more notice.

@martinjohndyer martinjohndyer added the -database Issues/PRs related to inserting into the database label Jul 23, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
-database Issues/PRs related to inserting into the database
Projects
None yet
Development

No branches or pull requests

1 participant