runzhiwang commented on pull request #1601: URL: https://github.com/apache/ozone/pull/1601#issuecomment-732697355
> > > Thanks for the review @runzhiwang . In a previous revision I had a strict check in `StateContext#addReport`: > > > [22846f4#diff-4e474a0cbb28091ed4fab8ee80878c3acf7e15711fba2a0d83bacd56ea8f5707L237-L240](https://github.com/apache/ozone/commit/22846f48ce34f3fcefa1de2d3bd97c4771c26742#diff-4e474a0cbb28091ed4fab8ee80878c3acf7e15711fba2a0d83bacd56ea8f5707L237-L240) > > > which ensures only `CommandStatusReports` and `IncrementalContainerReport` would ever be added to `reports` queue. Later this is removed since @avijayanhwx suggested we don't really need that. > > > The only two paths where reports would be queued are `addReports` and `StateContext#putBackReports`, where `HeartbeatEndpointTask#putBackReports` makes sure we only put back `CommandStatusReports` and `IncrementalContainerReport`. > > > Would you elaborate on the integration test plan you have on your mind? > > > > > > @smengcl OOM is a serious problem, so we need strong test. If somebody change the existed two path, or add the third path, and put ContainerReport in the queue, OOM will happen, so we need some restrain. The integration test maybe only can cover the existed two path, for example, you can decrease the time interval of ReportPublisher#call and increase the time interval of EndpointStateMachine.EndPointStates call(), so that ContainerReport generated many times but has not been send out, then we check the reports do not contain ContainerReport. > > Thanks @runzhiwang . > > I think a better and easier way to check this behavior is to add a new UT that calls `addReports` and `putBackReports` repetitively, then check the `reports` queue length. Should only expect the queue grow when report type is `CommandStatusReport` or `IncrementalContainerReport`, but not any other ones. What do you think? @smengcl I agree. ---------------------------------------------------------------- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: [email protected] --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
