fix(#4858): deduplicate events #4872
Conversation
| ]); | ||
| const incomingEvent = new InstanceEvent(message.data); | ||
| if (this.seenEventKeys.has(incomingEvent.key)) { | ||
| return; |
There was a problem hiding this comment.
I think this may be wrong here because then listOffset doesn't get updated properly.
@SteKoe @ulischulte your thoughts on this?
There was a problem hiding this comment.
You are right, but as far as I can see, the offset is not used in the view anyway. I will dig into that and check if it is still needed.
There was a problem hiding this comment.
listOffset is now respected and adjusted.
| export function deduplicateInstanceEvents(events: InstanceEvent[]) { | ||
| const seen = new Set<string>(); | ||
| return events.filter((event) => { | ||
| if (seen.has(event.key)) { |
There was a problem hiding this comment.
@SteKoe is the event key really enough?
It's imperative, in my opinion, that a SBA instance X shows the events recorded by the same instance and not, for example, by the instance Y.
Otherwise there is no traceability anymore and it's, therefore, better to keep things as they're now by enduring the duplicates.
There was a problem hiding this comment.
Since the backend does not contain the instance name of SBA in the events, this would be a bigger change than just adding deduplication in frontend.
As key is just using instanceId and version, no. We have to respect any value here. Event type as well as timestamp. Otherwise the following events would be reduced, even though they are not the same:
const events = [
{instance: '123', version: 1, type: "REGISTERED", timestamp: "678"},
{instance: '123', version: 1, type: "DEREGISTERED", timestamp: "890"},
]Thanks to the existing tests, I was able to extend the behavior and added some more use cases.
There was a problem hiding this comment.
But then you won't dedup anything I think
3 participants
No description provided.