[CARBONDATA-1900][Core,processing] Modify loadmetadata to store timestamp long value(in ms) instead of formatted date string for fields "loadStartTime" and "timestamp" #1666

mohammadshahidkhan · 2017-12-15T10:11:33Z

Problem:

If the table is moved to environment having different timezone or we change the system current timezone, after IUD operation some of the blocks are not treated as valid blocks.
[{"timestamp":"15-12-2017 16:50:31:703","loadStatus":"Success","loadName":"0","partitionCount":"0","isDeleted":"FALSE","dataSize":"912","indexSize":"700","updateDeltaEndTimestamp":"","updateDeltaStartTimestamp":"","updateStatusFileName":"","**loadStartTime**":"15-12-2017 16:50:27:493","visibility":"true","fileFormat":"COLUMNAR_V3"}]

part-0-0_batchno0-0-1513336827493.carbondata

If timezone is different than at the time load was done, the value calculated from loadStartTime "15-12-2017 16:50:27:493" will not match to the time stamp extracted from block file name.

Solution:

We should stop writing the loadStartTime and timestamp in "dd-MM-yyyy HH:mm:ss:SSS" format.
We should write the long value of the timestamp as given below.
[{"timestamp":"1513336827593","loadStatus":"Success","loadName":"0","partitionCount":"0","isDeleted":"FALSE","dataSize":"912","indexSize":"700","updateDeltaEndTimestamp":"","updateDeltaStartTimestamp":"","updateStatusFileName":"","loadStartTime":"1513336827493", "visibility":"true","fileFormat":"COLUMNAR_V3"}]

Any interfaces changed?
Any backward compatibility impacted?
For old loaded if string to Long parse fail, will fall back for date parsing.
Document update required?
For old loaded table we can add limitation that the table movement should be done
across the same timezone environment.
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.
Manually tested
For large changes, please consider breaking it into sub-tasks under an umbrella JIRA.
NA

CarbonDataQA · 2017-12-15T11:51:43Z

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

CarbonDataQA · 2017-12-15T13:10:26Z

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

ravipesala · 2017-12-15T15:39:01Z

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

mohammadshahidkhan · 2017-12-15T15:43:19Z

This PR is ok, no code change after first push only pushed the commit message change
@ravipesala Please review

CarbonDataQA · 2017-12-15T20:20:44Z

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

ravipesala · 2017-12-15T23:01:47Z

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

CarbonDataQA · 2017-12-16T00:20:53Z

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

mohammadshahidkhan · 2017-12-16T02:33:58Z

Failed Test in SDV build is unrelated, its randomly failing with other PR also,
but if re trigger getting passed.

mohammadshahidkhan · 2017-12-16T02:41:03Z

retest this please

CarbonDataQA · 2017-12-16T04:20:53Z

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

CarbonDataQA · 2017-12-16T04:20:54Z

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

jackylk · 2017-12-17T02:46:58Z

core/src/main/java/org/apache/carbondata/core/statusmanager/LoadMetadataDetails.java

-    } catch (ParseException e) {
-      LOGGER.error("Cannot convert" + loadStartTime + " to Time/Long type value" + e.getMessage());
-      return null;
+      // for new loads the factTimeStamp will be long string


In future for maintenance it is not easy to understand what new load is, can you add more background information for better maintenance. You can tell what is stored before Carbon 1.3 and what is changed since 1.3

Fixed Added the detailed comment at class level

Do not mention "new loads" in line 275. Add comment in line 279, mentioning it is the processing for existing table before carbon 1.3

CarbonDataQA · 2017-12-18T07:31:56Z

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

CarbonDataQA · 2017-12-18T07:33:26Z

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

ravipesala · 2017-12-18T07:49:56Z

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

jackylk · 2017-12-18T08:14:36Z

core/src/main/java/org/apache/carbondata/core/statusmanager/LoadMetadataDetails.java

-    } catch (ParseException e) {
-      LOGGER.error("Cannot convert" + factTimeStamp + " to Time/Long type value" + e.getMessage());
-      parser = new SimpleDateFormat(CarbonCommonConstants.CARBON_TIMESTAMP);
+      // for new loads the factTimeStamp will be long string


Do not mention "new loads" in line 275. Add comment in line 279, mentioning it is the processing for existing table before carbon 1.3

jackylk · 2017-12-18T08:15:39Z

core/src/main/java/org/apache/carbondata/core/statusmanager/LoadMetadataDetails.java

@@ -211,25 +234,32 @@ public long getLoadStartTimeAsLong() {
   * @return
   */
  private long convertTimeStampToLong(String factTimeStamp) {


What is the difference between this function and getTimeStamp, seems they are doing the same thing

This function getTimeStamp returns value in nano second and returns in mili second

ravipesala · 2017-12-18T08:29:49Z

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

…tamp long value(in ms) instead of formated date string for fields "loadStartTime" and "timestamp"

CarbonDataQA · 2017-12-18T09:53:38Z

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

CarbonDataQA · 2017-12-18T09:55:48Z

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

ravipesala · 2017-12-18T11:08:27Z

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

mohammadshahidkhan · 2017-12-19T06:51:37Z

retest this please

CarbonDataQA · 2017-12-19T07:19:23Z

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

CarbonDataQA · 2017-12-19T07:48:03Z

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

jackylk · 2017-12-19T10:11:56Z

LGTM

manishgupta88 · 2017-12-19T10:13:26Z

LGTM

…tamp long value(in ms) instead of formatted date string for fields "loadStartTime" and "timestamp" If the table is moved to environment having different timezone or we change the system current timezone, after IUD operation some of the blocks are not treated as valid blocks. We should stop writing the loadStartTime and timestamp in dd-MM-yyyy HH:mm:ss:SSS format. We should write the long value of the timestamp This closes apache#1666

mohammadshahidkhan force-pushed the tablestatus_timezone_fix branch from dbfd0db to b73a6df Compare December 15, 2017 15:05

mohammadshahidkhan changed the title ~~table status timezone problem fix~~ [CARBONDATA-1900][Core,processing] Modify loadmetadata to store timestamp long value(in ms) instead of formatted date string for fields "loadStartTime" and "timestamp" Dec 15, 2017

jackylk reviewed Dec 17, 2017

View reviewed changes

mohammadshahidkhan force-pushed the tablestatus_timezone_fix branch 2 times, most recently from c15fc73 to a2687b1 Compare December 18, 2017 07:02

jackylk reviewed Dec 18, 2017

View reviewed changes

[CARBONDATA-1900][Core,processing] Modify loadmetadata to store times…

2e66a7a

…tamp long value(in ms) instead of formated date string for fields "loadStartTime" and "timestamp"

mohammadshahidkhan force-pushed the tablestatus_timezone_fix branch from a2687b1 to 2e66a7a Compare December 18, 2017 09:04

asfgit closed this in 804ddb7 Dec 19, 2017

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[CARBONDATA-1900][Core,processing] Modify loadmetadata to store timestamp long value(in ms) instead of formatted date string for fields "loadStartTime" and "timestamp" #1666

[CARBONDATA-1900][Core,processing] Modify loadmetadata to store timestamp long value(in ms) instead of formatted date string for fields "loadStartTime" and "timestamp" #1666

mohammadshahidkhan commented Dec 15, 2017 •

edited

CarbonDataQA commented Dec 15, 2017

CarbonDataQA commented Dec 15, 2017

ravipesala commented Dec 15, 2017

mohammadshahidkhan commented Dec 15, 2017

CarbonDataQA commented Dec 15, 2017

ravipesala commented Dec 15, 2017

CarbonDataQA commented Dec 16, 2017

mohammadshahidkhan commented Dec 16, 2017 •

edited

mohammadshahidkhan commented Dec 16, 2017

CarbonDataQA commented Dec 16, 2017

CarbonDataQA commented Dec 16, 2017

jackylk Dec 17, 2017 •

edited

mohammadshahidkhan Dec 18, 2017

jackylk Dec 18, 2017

mohammadshahidkhan Dec 18, 2017

CarbonDataQA commented Dec 18, 2017

CarbonDataQA commented Dec 18, 2017

ravipesala commented Dec 18, 2017

jackylk Dec 18, 2017

mohammadshahidkhan Dec 18, 2017

jackylk Dec 18, 2017

mohammadshahidkhan Dec 18, 2017

ravipesala commented Dec 18, 2017

CarbonDataQA commented Dec 18, 2017

CarbonDataQA commented Dec 18, 2017

ravipesala commented Dec 18, 2017

mohammadshahidkhan commented Dec 19, 2017

CarbonDataQA commented Dec 19, 2017

CarbonDataQA commented Dec 19, 2017

jackylk commented Dec 19, 2017

manishgupta88 commented Dec 19, 2017

[CARBONDATA-1900][Core,processing] Modify loadmetadata to store timestamp long value(in ms) instead of formatted date string for fields "loadStartTime" and "timestamp" #1666

[CARBONDATA-1900][Core,processing] Modify loadmetadata to store timestamp long value(in ms) instead of formatted date string for fields "loadStartTime" and "timestamp" #1666

Conversation

mohammadshahidkhan commented Dec 15, 2017 • edited

CarbonDataQA commented Dec 15, 2017

CarbonDataQA commented Dec 15, 2017

ravipesala commented Dec 15, 2017

mohammadshahidkhan commented Dec 15, 2017

CarbonDataQA commented Dec 15, 2017

ravipesala commented Dec 15, 2017

CarbonDataQA commented Dec 16, 2017

mohammadshahidkhan commented Dec 16, 2017 • edited

mohammadshahidkhan commented Dec 16, 2017

CarbonDataQA commented Dec 16, 2017

CarbonDataQA commented Dec 16, 2017

jackylk Dec 17, 2017 • edited

Choose a reason for hiding this comment

mohammadshahidkhan Dec 18, 2017

Choose a reason for hiding this comment

jackylk Dec 18, 2017

Choose a reason for hiding this comment

mohammadshahidkhan Dec 18, 2017

Choose a reason for hiding this comment

CarbonDataQA commented Dec 18, 2017

CarbonDataQA commented Dec 18, 2017

ravipesala commented Dec 18, 2017

jackylk Dec 18, 2017

Choose a reason for hiding this comment

mohammadshahidkhan Dec 18, 2017

Choose a reason for hiding this comment

jackylk Dec 18, 2017

Choose a reason for hiding this comment

mohammadshahidkhan Dec 18, 2017

Choose a reason for hiding this comment

ravipesala commented Dec 18, 2017

CarbonDataQA commented Dec 18, 2017

CarbonDataQA commented Dec 18, 2017

ravipesala commented Dec 18, 2017

mohammadshahidkhan commented Dec 19, 2017

CarbonDataQA commented Dec 19, 2017

CarbonDataQA commented Dec 19, 2017

jackylk commented Dec 19, 2017

manishgupta88 commented Dec 19, 2017

mohammadshahidkhan commented Dec 15, 2017 •

edited

mohammadshahidkhan commented Dec 16, 2017 •

edited

jackylk Dec 17, 2017 •

edited