Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[CARBONDATA-1218] [GLOBAL SORT] In case of data-load failure the BadRecordsLogger.addRecordEntry map holding the task Status is not removing the task Entry. #1082

Conversation

mohammadshahidkhan
Copy link
Contributor

@mohammadshahidkhan mohammadshahidkhan commented Jun 23, 2017

Problem
For GLOBAL_SORT scope option in case of data-load failure the BadRecordsLogger.badRecordEntry map holding the task Status is not removing the task Entry.
Because of this the next load is getting failed even though the data being loaded has no bad records.
Solution
The map entry must be removed after load completion either success or fail.
Refactored the Bad record logger.

  • Any interfaces changed?
    NA
  • Any backward compatibility impacted?
    NA
  • Document update required?
    NA
  • Testing done
    Please provide details on
    - Whether new unit test cases have been added or why no new tests are required?
    - How it is tested? Please attach test report.
    - Is it a performance related change? Please attach the performance test report.
    - Any additional information to help reviewers in testing this change.
    Added Test case
  • For large changes, please consider breaking it into sub-tasks under an umbrella JIRA.
    NA

@asfgit
Copy link

asfgit commented Jun 23, 2017

Can one of the admins verify this patch?

@CarbonDataQA
Copy link

Build Success with Spark 1.6, Please check CI http://144.76.159.231:8080/job/ApacheCarbonPRBuilder/98/

@CarbonDataQA
Copy link

Build Success with Spark 2.1.0, Please check CI http://136.243.101.176:8080/job/ApacheCarbonPRBuilder/2670/

@asfgit
Copy link

asfgit commented Jun 28, 2017

Refer to this link for build results (access rights to CI server needed):
https://builds.apache.org/job/carbondata-pr-spark-1.6/691/

@CarbonDataQA
Copy link

Build Success with Spark 1.6, Please check CI http://144.76.159.231:8080/job/ApacheCarbonPRBuilder/183/

@CarbonDataQA
Copy link

Build Success with Spark 2.1.0, Please check CI http://136.243.101.176:8080/job/ApacheCarbonPRBuilder/2762/

@CarbonDataQA
Copy link

Build Failed with Spark 1.6, Please check CI http://144.76.159.231:8080/job/ApacheCarbonPRBuilder/232/

@CarbonDataQA
Copy link

Build Failed with Spark 2.1.0, Please check CI http://136.243.101.176:8080/job/ApacheCarbonPRBuilder/2812/

@CarbonDataQA
Copy link

Build Failed with Spark 2.1.0, Please check CI http://136.243.101.176:8080/job/ApacheCarbonPRBuilder/2822/

@CarbonDataQA
Copy link

Build Failed with Spark 1.6, Please check CI http://144.76.159.231:8080/job/ApacheCarbonPRBuilder/242/

@CarbonDataQA
Copy link

Build Success with Spark 2.1.0, Please check CI http://136.243.101.176:8080/job/ApacheCarbonPRBuilder/2823/

@CarbonDataQA
Copy link

Build Success with Spark 1.6, Please check CI http://144.76.159.231:8080/job/ApacheCarbonPRBuilder/243/

@CarbonDataQA
Copy link

Can one of the admins verify this patch?

@CarbonDataQA
Copy link

Build Success with Spark 2.1.0, Please check CI http://136.243.101.176:8080/job/ApacheCarbonPRBuilder1/52/

@ravipesala
Copy link
Contributor

SDV Build Fail , Please check CI http://144.76.159.231:8080/job/ApacheSDVTests/1228/

@CarbonDataQA
Copy link

Build Success with Spark 2.1.0, Please check CI http://136.243.101.176:8080/job/ApacheCarbonPRBuilder1/597/

@mohammadshahidkhan mohammadshahidkhan changed the title [CARBONDATA-1218] In case of data-load failure the BadRecordsLogger.adRecordEntry map holding the task Status is not removing the task Entry. [WIP][CARBONDATA-1218] In case of data-load failure the BadRecordsLogger.adRecordEntry map holding the task Status is not removing the task Entry. Nov 28, 2017
@mohammadshahidkhan mohammadshahidkhan changed the title [WIP][CARBONDATA-1218] In case of data-load failure the BadRecordsLogger.adRecordEntry map holding the task Status is not removing the task Entry. [CARBONDATA-1218] In case of data-load failure the BadRecordsLogger.adRecordEntry map holding the task Status is not removing the task Entry. Dec 16, 2017
@CarbonDataQA
Copy link

Build Success with Spark 2.2.0, Please check CI http://88.99.58.216:8080/job/ApacheCarbonPRBuilder/832/

@CarbonDataQA
Copy link

Build Failed with Spark 2.1.0, Please check CI http://136.243.101.176:8080/job/ApacheCarbonPRBuilder1/2059/

@CarbonDataQA
Copy link

Build Failed with Spark 2.2.0, Please check CI http://88.99.58.216:8080/job/ApacheCarbonPRBuilder/837/

@ravipesala
Copy link
Contributor

SDV Build Success , Please check CI http://144.76.159.231:8080/job/ApacheSDVTests/2368/

@CarbonDataQA
Copy link

Build Failed with Spark 2.1.0, Please check CI http://136.243.101.176:8080/job/ApacheCarbonPRBuilder1/2063/

@mohammadshahidkhan
Copy link
Contributor Author

retest this please

@CarbonDataQA
Copy link

Build Failed with Spark 2.2.0, Please check CI http://88.99.58.216:8080/job/ApacheCarbonPRBuilder/871/

@CarbonDataQA
Copy link

Build Failed with Spark 2.1.0, Please check CI http://136.243.101.176:8080/job/ApacheCarbonPRBuilder1/2096/

@CarbonDataQA
Copy link

Build Success with Spark 2.2.0, Please check CI http://88.99.58.216:8080/job/ApacheCarbonPRBuilder/914/

@ravipesala
Copy link
Contributor

SDV Build Fail , Please check CI http://144.76.159.231:8080/job/ApacheSDVTests/2427/

@CarbonDataQA
Copy link

Build Success with Spark 2.1.0, Please check CI http://136.243.101.176:8080/job/ApacheCarbonPRBuilder1/2144/

@mohammadshahidkhan
Copy link
Contributor Author

retest this please

@ravipesala
Copy link
Contributor

SDV Build Success , Please check CI http://144.76.159.231:8080/job/ApacheSDVTests/2471/

@mohammadshahidkhan mohammadshahidkhan force-pushed the task_status_cleanup_problem branch 2 times, most recently from 6b95d4a to 81741eb Compare January 2, 2018 07:03
@mohammadshahidkhan mohammadshahidkhan changed the title [CARBONDATA-1218] In case of data-load failure the BadRecordsLogger.adRecordEntry map holding the task Status is not removing the task Entry. [CARBONDATA-1218] GLOBAL SORT, In case of data-load failure the BadRecordsLogger.addRecordEntry map holding the task Status is not removing the task Entry. Jan 2, 2018
@mohammadshahidkhan mohammadshahidkhan changed the title [CARBONDATA-1218] GLOBAL SORT, In case of data-load failure the BadRecordsLogger.addRecordEntry map holding the task Status is not removing the task Entry. [CARBONDATA-1218] [GLOBAL SORT] In case of data-load failure the BadRecordsLogger.addRecordEntry map holding the task Status is not removing the task Entry. Jan 2, 2018
@CarbonDataQA
Copy link

Build Failed with Spark 2.1.0, Please check CI http://136.243.101.176:8080/job/ApacheCarbonPRBuilder1/2470/

@CarbonDataQA
Copy link

Build Success with Spark 2.2.0, Please check CI http://88.99.58.216:8080/job/ApacheCarbonPRBuilder/1246/

@CarbonDataQA
Copy link

Build Success with Spark 2.1.0, Please check CI http://136.243.101.176:8080/job/ApacheCarbonPRBuilder1/2473/

@ravipesala
Copy link
Contributor

SDV Build Success , Please check CI http://144.76.159.231:8080/job/ApacheSDVTests/2650/

@CarbonDataQA
Copy link

Build Success with Spark 2.2.0, Please check CI http://88.99.58.216:8080/job/ApacheCarbonPRBuilder/1249/

@ravipesala
Copy link
Contributor

SDV Build Success , Please check CI http://144.76.159.231:8080/job/ApacheSDVTests/2653/

assert(true)
sql("drop table IF EXISTS loadIssue")
}

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  1. Use only one try and finally block inside test code..try should start from first line and end at last and finally should only reset the carbon property
  2. Instead of try catch use intercept[Exception] only for the 1st case where exception is expected. For 2nd case try catch is not required...if exception comes anyways test case will fail

// rename the bad record in progress to normal
CarbonTableIdentifier identifier =
configuration.getTableIdentifier().getCarbonTableIdentifier();
CarbonDataProcessorUtil.renameBadRecordsFromInProgressToNormal(configuration,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  1. Avoid calling the method using classname. Method is present in the same class.
  2. I think we can introduce a new class called BadRecordsUtil which will have all the functions related to bad records like
    renameBadRecord, hasBadRecord, renameBadRecordsFromInProgressToNormal (make it private in the new util class)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Refactored

…adRecordEntry map holding the task Status is not removing the task Entry
@CarbonDataQA
Copy link

Build Failed with Spark 2.1.0, Please check CI http://136.243.101.176:8080/job/ApacheCarbonPRBuilder1/2554/

@ravipesala
Copy link
Contributor

SDV Build Success , Please check CI http://144.76.159.231:8080/job/ApacheSDVTests/2720/

@CarbonDataQA
Copy link

Build Success with Spark 2.2.1, Please check CI http://88.99.58.216:8080/job/ApacheCarbonPRBuilder/1331/

@CarbonDataQA
Copy link

Build Success with Spark 2.1.0, Please check CI http://136.243.101.176:8080/job/ApacheCarbonPRBuilder1/2567/

@ravipesala
Copy link
Contributor

SDV Build Success , Please check CI http://144.76.159.231:8080/job/ApacheSDVTests/2731/

@asfgit asfgit closed this in 837fdd2 Jan 5, 2018
anubhav100 pushed a commit to anubhav100/incubator-carbondata that referenced this pull request Jun 22, 2018
…adRecordEntry map holding the task Status is not removing the task Entry

Problem
For GLOBAL_SORT scope option in case of data-load failure the BadRecordsLogger.badRecordEntry map holding the task Status is not removing the task Entry.
Because of this the next load is getting failed even though the data being loaded has no bad records.

Solution
The map entry must be removed after load completion either success or fail.
Refactored the Bad record logger.

This closes apache#1082
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants