Skip to content

Conversation

ramaraochavali
Copy link
Contributor

Signed-off-by: Rama rama.rao@salesforce.com
Description: Reverting the empty stat update fix so that we fix this bug #4485
Risk Level: Low
Testing: Automated tests
Docs Changes: N/A
Release Notes: N/A
Fixes #4485

Signed-off-by: Rama <rama.rao@salesforce.com>
@ramaraochavali
Copy link
Contributor Author

@htuch reverted that fix

@htuch
Copy link
Member

htuch commented Sep 21, 2018

@ramaraochavali thanks for this. @rfaulk can you confirm this fixes the issue (if you can't patch it onto the internal tree, just make the changes by hand, it's small)?

I'd prefer not to make too many changes to how these stats are computed, as consumers will see anomalies. While @rfaulk confirms, maybe we can try and figure out the right long term fix. I would argue as a strawman that the previous "empty stat bump" could be considered correct once we have incremental xDS, since the right way to provide partial updates will be via that mechanism. Does that sound too crazy?

@ramaraochavali
Copy link
Contributor Author

@htuch I agree that "empty stat bump" could be correct with incremental xDS. I am fine to wait on this till increments xDS comes and we can fix it if it is still confusing then.

@htuch
Copy link
Member

htuch commented Sep 21, 2018

@ramaraochavali OK. Let's wait for @rfaulk to confirm that this does fix the bug and we can ship the revert. Thanks.

@htuch htuch self-assigned this Sep 21, 2018
@ramaraochavali
Copy link
Contributor Author

Ok. No problem. I was able to reproduce the problem locally and this change fixed it. So we should be good.

@htuch
Copy link
Member

htuch commented Sep 23, 2018

OK, SGTM, will merge, thanks.

@htuch htuch merged commit 087c1a1 into envoyproxy:master Sep 23, 2018
@ramaraochavali ramaraochavali deleted the fix/revert_stats_fix branch September 24, 2018 03:51
ramaraochavali added a commit to ramaraochavali/envoy that referenced this pull request Mar 4, 2019
…d because of envoyproxy#4485 via envoyproxy#4490

Signed-off-by: Rama Chavali <rama.rao@salesforce.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Empty EDS prevents LDS and RDS requests
2 participants