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

[HOTFIX] Concurrent insert test case failure fix #3610

Closed

Conversation

kunal642
Copy link
Contributor

@kunal642 kunal642 commented Feb 11, 2020

Why is this PR needed?

System.currentTimeMillis() is giving same results for 2 insert due to which 1 load is deleting the temp folder for another load.

What changes were proposed in this PR?

  1. Change to System.nanoTime()

Does this PR introduce any user interface change?

  • No
  • Yes. (please explain the change and update document)

Is any new testcase added?

  • No
  • Yes

@kunal642 kunal642 force-pushed the concurrent_insert_testcase_fix branch from ca152b0 to 7471f26 Compare February 11, 2020 06:36
@CarbonDataQA1
Copy link

Build Success with Spark 2.4.4, Please check CI http://121.244.95.60:12545/job/ApacheCarbon_PR_Builder_2.4.4/228/

@@ -290,7 +290,7 @@ case class CarbonLoadDataCommand(
FileFactory.mkdirs(metadataDirectoryPath)
}
} else {
carbonLoadModel.setSegmentId(System.currentTimeMillis().toString)
carbonLoadModel.setSegmentId(System.nanoTime().toString)
Copy link
Member

Choose a reason for hiding this comment

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

Many times have observed (mainly from user environment) that , two concurrent tasks can get same nano time. So, we use UUID in these scenario. But for segmentID we cannot use as it may be casted to long.

Need other solutions or go back to old look up logic before #3601 for non-transactional tables

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Added lookup for non-transactional also..

@CarbonDataQA1
Copy link

Build Success with Spark 2.3.4, Please check CI http://121.244.95.60:12545/job/ApacheCarbonPRBuilder2.3/1930/

@kunal642 kunal642 force-pushed the concurrent_insert_testcase_fix branch from 7471f26 to 6a39669 Compare February 11, 2020 08:29
@CarbonDataQA1
Copy link

Build Success with Spark 2.4.4, Please check CI http://121.244.95.60:12545/job/ApacheCarbon_PR_Builder_2.4.4/231/

@CarbonDataQA1
Copy link

Build Success with Spark 2.3.4, Please check CI http://121.244.95.60:12545/job/ApacheCarbonPRBuilder2.3/1933/

@kunal642
Copy link
Contributor Author

retest this please

@CarbonDataQA1
Copy link

Build Success with Spark 2.4.4, Please check CI http://121.244.95.60:12545/job/ApacheCarbon_PR_Builder_2.4.4/233/

@CarbonDataQA1
Copy link

Build Success with Spark 2.3.4, Please check CI http://121.244.95.60:12545/job/ApacheCarbonPRBuilder2.3/1935/

@ajantha-bhat
Copy link
Member

LGTM

@asfgit asfgit closed this in ca19958 Feb 11, 2020
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

3 participants