You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@flink.apache.org by chiwanpark <gi...@git.apache.org> on 2015/02/08 08:54:59 UTC

[GitHub] flink pull request: [FLINK-1179] Add button to JobManager web inte...

GitHub user chiwanpark opened a pull request:

    https://github.com/apache/flink/pull/374

    [FLINK-1179] Add button to JobManager web interface to request stack trace of a TaskManager

    This PR contains following changes:
    * Add public constructors of `org.apache.flink.runtime.instance.InstanceID` for sending instance ID from web interface to job manager
    * Add a helper method called `getRegisteredInstanceById(InstanceID)` into `org.apache.flink.runtime.instance.InstanceManager` for finding Akka Actor from instance ID
    * Add akka messages called `RequestStackTrace`, `SendStackTrace` and `StackTrace`
    * Modify a task manager page in web interface of job manager to request and show stack trace of a task manager
    
    The following image is a screenshot of web interface of job manager.
    
    ![screen shot 2015-02-08 at 3 49 51 pm](https://cloud.githubusercontent.com/assets/1941681/6095765/9293e996-afaf-11e4-9e8e-4dcd69ce595b.png)


You can merge this pull request into a Git repository by running:

    $ git pull https://github.com/chiwanpark/flink FLINK-1179

Alternatively you can review and apply these changes as the patch at:

    https://github.com/apache/flink/pull/374.patch

To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:

    This closes #374
    
----
commit 83ab236ce52dd8c3b0aa0b94ee8644a4de28e152
Author: Chiwan Park <ch...@icloud.com>
Date:   2015-02-08T06:36:19Z

    [FLINK-1179] [runtime] Add helper method for InstanceID

commit a2a0a0f8261851c93330417f3c60a16c5f1d2dd5
Author: Chiwan Park <ch...@icloud.com>
Date:   2015-02-08T07:05:29Z

    [FLINK-1179] Add internal API for obtaining StackTrace

commit 423e64ca4cc6ad4a9396e4418eab95ce5b81b219
Author: Chiwan Park <ch...@icloud.com>
Date:   2015-02-08T07:10:03Z

    [FLINK-1179] [jobmanager] Add button to JobManager web interface to request stack trace of a TaskManager

----


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] flink pull request: [FLINK-1179] Add button to JobManager web inte...

Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:

    https://github.com/apache/flink/pull/374


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] flink pull request: [FLINK-1179] Add button to JobManager web inte...

Posted by tillrohrmann <gi...@git.apache.org>.
Github user tillrohrmann commented on a diff in the pull request:

    https://github.com/apache/flink/pull/374#discussion_r24298309
  
    --- Diff: flink-runtime/src/main/scala/org/apache/flink/runtime/taskmanager/TaskManager.scala ---
    @@ -300,6 +300,16 @@ import scala.collection.JavaConverters._
         case LogMemoryUsage =>
           logMemoryStats()
     
    +    case SendStackTrace =>
    +      val traces = Thread.getAllStackTraces.asScala
    +      val stackTraceStr = traces.map((trace: (Thread, Array[StackTraceElement])) => {
    +        val (thread, elements) = trace
    +        val traceStr = elements.map((trace: StackTraceElement) => trace.toString).mkString("\n")
    --- End diff --
    
    I think that you don't need the map operation here. ```elements.mkString("\n")``` should do the same.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] flink pull request: [FLINK-1179] Add button to JobManager web inte...

Posted by rmetzger <gi...@git.apache.org>.
Github user rmetzger commented on the pull request:

    https://github.com/apache/flink/pull/374#issuecomment-74387918
  
    .... Merging this now ... https://github.com/rmetzger/flink/compare/FLINK-1179


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] flink pull request: [FLINK-1179] Add button to JobManager web inte...

Posted by StephanEwen <gi...@git.apache.org>.
Github user StephanEwen commented on the pull request:

    https://github.com/apache/flink/pull/374#issuecomment-73477910
  
    Very nice work. I have one comment inline, otherwise +1 to go!


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] flink pull request: [FLINK-1179] Add button to JobManager web inte...

Posted by StephanEwen <gi...@git.apache.org>.
Github user StephanEwen commented on the pull request:

    https://github.com/apache/flink/pull/374#issuecomment-74068773
  
    Looks good. There is a small conflict with #384 , but we can try and fix this while merging.
    
    +1 


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] flink pull request: [FLINK-1179] Add button to JobManager web inte...

Posted by tillrohrmann <gi...@git.apache.org>.
Github user tillrohrmann commented on the pull request:

    https://github.com/apache/flink/pull/374#issuecomment-73409662
  
    Hi Chiwan, thanks for your work. It looks really good. I had some just some minor remarks.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] flink pull request: [FLINK-1179] Add button to JobManager web inte...

Posted by rmetzger <gi...@git.apache.org>.
Github user rmetzger commented on the pull request:

    https://github.com/apache/flink/pull/374#issuecomment-74310825
  
    I'll probably merge this change tomorrow because I'm working on a bigger change on the web frontend.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] flink pull request: [FLINK-1179] Add button to JobManager web inte...

Posted by StephanEwen <gi...@git.apache.org>.
Github user StephanEwen commented on a diff in the pull request:

    https://github.com/apache/flink/pull/374#discussion_r24315701
  
    --- Diff: flink-runtime/src/main/java/org/apache/flink/runtime/jobmanager/web/SetupInfoServlet.java ---
    @@ -127,25 +134,42 @@ private void writeTaskmanagers(HttpServletResponse resp) throws IOException {
     				objInner.put("physicalMemory", instance.getResources().getSizeOfPhysicalMemory() >>> 20);
     				objInner.put("freeMemory", instance.getResources().getSizeOfJvmHeap() >>> 20);
     				objInner.put("managedMemory", instance.getResources().getSizeOfManagedMemory() >>> 20);
    +				objInner.put("instanceID", instance.getId());
     				array.put(objInner);
     			}
     			catch (JSONException e) {
     				LOG.warn("Json object creation failed", e);
     			}
    -			
    +
     		}
     		try {
     			obj.put("taskmanagers", array);
     		} catch (JSONException e) {
     			LOG.warn("Json object creation failed", e);
     		}
    -		
    +
     		PrintWriter w = resp.getWriter();
     		w.write(obj.toString());
     	}
    -	
    +
    +	private void writeStackTraceOfTaskManager(String instanceIdStr, HttpServletResponse resp) throws IOException {
    --- End diff --
    
    The `RequestStackTrace` message may fail, if the task manager is not reachable.
    
    I suggest to surround this block with try / catch(Throwable) and forward the error message to the web client.
    
    The response JSON may then have two fields: "errorMessage" and "stackTrace". If "errorMessage" is defined, display the message, otherwise print the stack trace. 


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] flink pull request: [FLINK-1179] Add button to JobManager web inte...

Posted by tillrohrmann <gi...@git.apache.org>.
Github user tillrohrmann commented on a diff in the pull request:

    https://github.com/apache/flink/pull/374#discussion_r24298303
  
    --- Diff: flink-runtime/src/main/scala/org/apache/flink/runtime/jobmanager/JobManager.scala ---
    @@ -349,6 +349,11 @@ Actor with ActorLogMessages with ActorLogging {
         case Heartbeat(instanceID) =>
           instanceManager.reportHeartBeat(instanceID)
     
    +    case RequestStackTrace(instanceID) =>
    +      val taskManager = instanceManager.getRegisteredInstanceById(instanceID).getTaskManager
    +      val result = AkkaUtils.ask[StackTrace](taskManager, SendStackTrace)
    --- End diff --
    
    This is a blocking call within the actor thread. We should avoid this. You can simply forward the ```SendStackTrace``` message to the respective TaskManager: ```taskManager forward SendStacktrace```


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] flink pull request: [FLINK-1179] Add button to JobManager web inte...

Posted by chiwanpark <gi...@git.apache.org>.
Github user chiwanpark commented on the pull request:

    https://github.com/apache/flink/pull/374#issuecomment-73410413
  
    @tillrohrmann Thanks for your advice. I will fix it!


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] flink pull request: [FLINK-1179] Add button to JobManager web inte...

Posted by chiwanpark <gi...@git.apache.org>.
Github user chiwanpark commented on the pull request:

    https://github.com/apache/flink/pull/374#issuecomment-73509674
  
    @StephanEwen Thanks for your advice! I fixed it.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] flink pull request: [FLINK-1179] Add button to JobManager web inte...

Posted by rmetzger <gi...@git.apache.org>.
Github user rmetzger commented on the pull request:

    https://github.com/apache/flink/pull/374#issuecomment-73478511
  
    I've tried it out locally. Looks very nice. Thank you.
    
    +1 to merge.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---