Fix potential problems and AQE updates in Qual tool

[amahussein]

Signed-off-by: Ahmed Hussein (amahussein) [email protected]

Fixes #1019

What this PR fixes:

• Fixes a bug introduced by Refactor TaskEnd to be accessible by Q/P tools #1000 where the Potential Problems of file sql_duration_and_executor_cpu_time_percent.csv generated by CSV file would be empty.
• Fixes an old bug in the Qual tool caused by processing SQLplan as they are being created "SQLExecutionStart" and AQEUpdate. this bug was leaving some duplicate records in the SQL-to-problematic

Changes:

• In order to fix the Old Bug caused by AQEs:
  • Avoid processing SQLPlans by the event handler. Instead, process the plan after all the eventlogs are processed.
  • This implies removing QualificationAppInfo.processSQLPlan since the logic is the same as AppSQLPlanAnalyzer.processSQLPlanMetrics()
  • Fix the implementation of RunningQualificationApp to make sure that QualificationAppInfo.processSQLPlan is called before aggregateStats(). Otherwise, the RunningQualificationApp would have empty dataSources/problematics/writeDataFormats
• In order to fix the empty column:
  • Broke the logic of AppSQLPlanAnalyzer.processSQLPlanMetrics() into visitNode() that is separate from the main loop.
  • Created QualSQLPlanAnalyzer that extends AppSQLPlanAnalyzer overriding the visitNode() to be able to update the WriteDataFromats
• Misc changes:
  • changed Some datastructure types to preserve the order of insertion or order of Keys.
  • For unit-tests: Updated the order of the unsupported execs in the expected Qualification Execs.

[tgravescs]

why is appIndex not passed in here?

[amahussein]

Changed the code to pass appIndex to make it more readable.
The reason it was not passing appIndex before is that it is handled by AppSQLPlanAnalyzer.apply().

    val sqlAnalyzer = app match {
      case qApp: QualificationAppInfo =>
        new QualSQLPlanAnalyzer(qApp, appIndex)
      case pApp: ApplicationInfo =>
        new AppSQLPlanAnalyzer(pApp, pApp.index)
    }

[amahussein]

Thanks @tgravescs
I addressed the comments.

[amahussein]

Changed the code to pass appIndex to make it more readable.
The reason it was not passing appIndex before is that it is handled by AppSQLPlanAnalyzer.apply().

    val sqlAnalyzer = app match {
      case qApp: QualificationAppInfo =>
        new QualSQLPlanAnalyzer(qApp, appIndex)
      case pApp: ApplicationInfo =>
        new AppSQLPlanAnalyzer(pApp, pApp.index)
    }

[parthosa]

Does this have any performance impact and do we have a tracking issue or need one?

[amahussein]

There is indeed a memory overhead because of allocating more objects in memory. This overhead was introduced in #1000; but that was the price to combine the 2 tools.
Depending on priorities, we will address the redundant information stored in QualificationAppInfo

• We do have [FEA] Improve performance of core module #367 as an umbrella for performance issues.
  • There is a plan to set the dataBase storage to be a summer project for interns.
  • there is Refactor core to converge Qualification and Profiling Tools implementation #980 which is still open and we can add subtask as we think is necessary to consider that done.
• The incremental refactor changes the code frequently. I find that filing issues for each possible improvement will be cumbersome and create of flood of overlapping issues. So, unless there is a bug, I mark possible improvements as TODO that we can later revisit depending on priorities.

[parthosa]

nit: typo Should this be 'SQLID'?