-
Notifications
You must be signed in to change notification settings - Fork 712
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
Incorrect number of connections shown on internet nodes #1495
Comments
This will be the table ids not being unique again... |
@tomwilkie please elaborate. Is this a f/e or b/e issue? What I am seeing in e.g. "Weave Cloud (Prod)" is that the 'show more' count is something like "+1126", but expanding the list only adds about 120 items. The other thing that is odd here is that only 2 items are shown by default - looking at the code, the default should be 5. All that makes me think that it is a f/e issue. @davkal @foot |
@tomwilkie Correct. This effect can be seen by the first 5 shown (only 2 are rendered because the other 3 are duplicates) and the collapsed ones (there seemed to be 120 unique rows among the 1126 entries). We're using a row's |
I find it hard to believe that we actually have 1126 connections to AWS services / the internet in prod. 120 is a more plausible figure. perhaps the data contains genuine duplicates? |
For counting we were using a table keyed on a struct containing Node pointers. For connections between ordinary nodes this works just fine. But for connections to/from the Internet node we want to track individual address/port combinations, which involves an extra lookup. Since our data structures generally contain values, not pointers, this produces aliases. As a result n connections from/to a node to/from a specific Internet IP+port would result in n rows in the count table, each with a count of 1, instead of one row with a count of n. Things wouldn't be so bad if it was actually rendered like that - annoying, but at least accurate - but... Each row has an ID which is computed from the node IDs and ports. Not Node references. The ID must be unique - the frontend will only render *one* thing per ID. Since the row IDs of our n rows are all the same, we see one row with a count of 1 instead of n rows with a count of 1. Furthermore, since the frontend's table row limiting is counting rows, not unique row IDs, a) fewer rows would be rendered than expected, and b) the displayed count of the number of extra rows would be too high. The fix is to replace the Node pointers in the key with Node IDs. This does require an extra table lookup when we come to produce the rows, but that is just a fairly cheap map lookup. Fixes #1495.
fix internet connection counts Fixes #1495.
The +17 is wrong:
The text was updated successfully, but these errors were encountered: