FOGL-8285 : Added new JSONB field in streams table#1455
FOGL-8285 : Added new JSONB field in streams table#1455
Conversation
Signed-off-by: nandan <nandan.ghildiyal@gmail.com>
Signed-off-by: nandan <nandan.ghildiyal@gmail.com>
|
Have we run all the system tests against this branch? |
Only system tests have been executed against it. Will run labscripts and verify for upgrade case as well |
I executed system tests, lab scripts using CI Jobs and did some manual testing. Please refer following sheet for detailed report https://docs.google.com/spreadsheets/d/1m5JEiAd1BBo4FTlvuTVI1f_F1tON8E3X7P2bdqXhfbo/edit?usp=sharing |
MarkRiddoch
left a comment
MarkRiddoch
left a comment
There was a problem hiding this comment.
This change concerns me a lot. It is such an extrenely unlikely thing for a real user to ever do, change the data source they are sending to a given destination after they have sent data already, that I woud be more inclined to disallow the change rather than put code it that tries to deal with it. Especially when this is a very sensitive area of the code. It controls the dat asent to a given destination and is responsible for making sure we don't miss data or resed data. At the very least it requires considerable testing of lots of different scenarios.
My feeing is we do not make this change.
| /** | ||
| * Migrate data of last_object field of streams table to new field last_objects | ||
| */ | ||
| void DataLoad::migrateStats() |
There was a problem hiding this comment.
This is nothing to do with stats, it is the position in the stream. The object ID's in a stream are not by defintion continuous, therefore making the assumption they are is dangerous. Consider for example cases of filter pipelines in the north that remove readings fom the stream.
MarkRiddoch
left a comment
MarkRiddoch
left a comment
There was a problem hiding this comment.
I think the upgrade is not good enough to apply to a real system that might be sending north and will replay readings, which it should not do. The issue that is trying to be fixed here is very niche and probably not one that would be encountered or is even worth trying to deal with. It is probably better to make the source immutable once created and force a new north to be created if you want to send different data.
| @@ -0,0 +1 @@ | |||
| ALTER TABLE fledge.streams ADD COLUMN last_objects jsonb NOT NULL DEFAULT '{"Readings":0,"Stats":0,"Audit":0}'::jsonb; | |||
There was a problem hiding this comment.
The is not preserving the current point reached in the readings being sent north. This will be an issue for upgrade of a running system.
| @@ -0,0 +1 @@ | |||
| ALTER TABLE fledge.streams ADD COLUMN last_objects JSON NOT NULL DEFAULT '{"Readings":0,"Stats":0,"Audit":0}'; | |||
There was a problem hiding this comment.
As above, the current position is lost so this will cause issues.
| @@ -0,0 +1 @@ | |||
| ALTER TABLE fledge.streams ADD COLUMN last_objects JSON NOT NULL DEFAULT '{"Readings":0,"Stats":0,"Audit":0}'; | |||
There was a problem hiding this comment.
As with other upgrader scripts, this does not preserve the location in the readings
2 participants
This implementation is done only for north service. Will implement changes for north task later