[jira] [Commented] (PIG-4714) Improve logging across multiple components with callerId

["Rohini Palaniswamy (JIRA)" <ji...@apache.org>]

    [ https://issues.apache.org/jira/browse/PIG-4714?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15289742#comment-15289742 ] 

Rohini Palaniswamy commented on PIG-4714:
-----------------------------------------

Few comments:
   1) pig.ats.enabled=true is the default and this code affects PigServer. We should have it false as most folks won't have ATS enabled with Hadoop 2.x. You should also additionally check the yarn setting to see if timeline service is enabled even if pig setting is turned on similar to Tez. If it is turned off, then code with throw exception.
conf.getBoolean(YarnConfiguration.TIMELINE_SERVICE_ENABLED,
      YarnConfiguration.DEFAULT_TIMELINE_SERVICE_ENABLED)

  2) timelineClient.stop(); called twice in shutdown hook. One after catch block should be removed.
  3) Can we have the static constant names in all caps and moved to the top before non-static ones.
+    public static final String EntityType = "PIG_SCRIPT_ID";
+    public static final String EntityCallerId = "callerId";
+    public static final String CallerContext = "PIG";
  4) 
static ATSService instance; -  Should be private
private static ExecutorService executor; - This should be non-static
 5) Someplaces it is static public. Can we have it public static in all places.
 6) Can we get rid of Object LOCK = new Object(); ? The getInstance() method should be synchronized and not the constructor. Else it can still create two copies of ATSService
 7) We should just call executor.shutdownNow instead of awaitTermination. There are already lot of shutdown hooks in Pig and others like hdfs. Have seen issues were some shutdown hooks take time and prevent others from executing before NodeManager sends sigterm. So we should avoid doing  awaitTermination even if it is 3 seconds. And since we are calling it at beginning of script execution I don't think logging of any events would still be pending. 
 8) public static final String CALLER_ID = "pig.log.trace.id";
  This is not being set anywhere. Will always be null
 9) public class ATSService
   - Can we call it PigATSClient as it is a client and not a service? 
 10) log.warn("Error posting to ATS: " + e.getMessage());
   Might be good to have full stack trace logged to see what is the issue in case it fails.
 

> Improve logging across multiple components with callerId
> --------------------------------------------------------
>
>                 Key: PIG-4714
>                 URL: https://issues.apache.org/jira/browse/PIG-4714
>             Project: Pig
>          Issue Type: Improvement
>          Components: impl
>            Reporter: Daniel Dai
>            Assignee: Daniel Dai
>             Fix For: 0.16.0
>
>         Attachments: PIG-4714-1.patch, PIG-4714-2.patch
>
>
> The idea is to add callerId to every component, so we can track the chain of application which cause the underlining operation. A typical chain is Oozie->Pig->Tez->Hdfs. With proper callerId logging, we can trace Hdfs operation back to Oozie workflow which triggers it.
> The protocol we decided is every component log its immediate callerId. 
> For Pig, this includes passing Pig script ID to underlining components as callerId using component specific API, log callerId of Pig and store it on ATS. More specific, it includes:
> 1. Generate a CallerId for each Pig script, pass it to Hdfs/Yarn/MapReduce/Tez which Pig invokes
> 2. Pig caller passes "pig.log.trace.id" to Pig, Pig will publish it to ATS



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)