slfan1989 commented on code in PR #8652:
URL: https://github.com/apache/hadoop/pull/8652#discussion_r3724586639
##########
hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-app/src/main/java/org/apache/hadoop/mapreduce/v2/app/webapp/AMWebApp.java:
##########
@@ -38,6 +40,7 @@ public AMWebApp(AppContext appContext) {
@Override
public void setup() {
+ bind(App.class).in(Singleton.class);
bind(AppContext.class).toInstance(appContext);
Review Comment:
Thanks for fixing the attempts page. The App binding should be
request-scoped rather than singleton-scoped.
App contains mutable request-specific state through setJob() and setTask().
Binding it as a Singleton allows concurrent or subsequent requests to overwrite
or reuse another request's job/task state. The controller and the rendered view
need to share the same App instance only within a single HTTP request.
Could you please use:
```
bind(App.class).in(RequestScoped.class);
```
with com.google.inject.servlet.RequestScoped?
It would also be helpful to add a regression test that starts the AM WebApp
and requests /mapreduce/attempts/<job_id>/m/SUCCESSFUL, since the existing page
tests do not exercise the real AMWebApp Guice lifecycle.
--
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.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
