You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@lens.apache.org by Lavkesh Lahngir <la...@gmail.com> on 2017/01/19 12:08:33 UTC
Re: Review Request 55712: Fact Schema change to support all update
periods in one storage
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/55712/
-----------------------------------------------------------
(Updated Jan. 19, 2017, 12:08 p.m.)
Review request for lens.
Summary (updated)
-----------------
Fact Schema change to support all update periods in one storage
Bugs: LENS-1386
https://issues.apache.org/jira/browse/LENS-1386
Repository: lens
Description (updated)
-------
A new data structure XUpdatePeriodTableDescriptor is introduced which contains an update period and table descriptor. Now the XUpdatePeriods will contain a list of XUpdatePeriodTableDescriptor or XUpdatePeriod
Diffs
-----
lens-api/src/main/resources/cube-0.1.xsd f438f48
lens-cube/src/main/java/org/apache/lens/cube/metadata/CubeFactTable.java adb6c92
lens-cube/src/main/java/org/apache/lens/cube/metadata/CubeMetastoreClient.java 6c9cde2
lens-cube/src/main/java/org/apache/lens/cube/metadata/MetastoreUtil.java 53cf8af
lens-server/src/main/java/org/apache/lens/server/metastore/CubeMetastoreServiceImpl.java 8b10d1d
lens-server/src/main/java/org/apache/lens/server/metastore/JAXBUtils.java 51fcb43
Diff: https://reviews.apache.org/r/55712/diff/
Testing
-------
Thanks,
Lavkesh Lahngir
Re: Review Request 55712: Fact Schema change to support all update
periods in one storage
Posted by Amareshwari Sriramadasu <am...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/55712/#review162386
-----------------------------------------------------------
I see we are creating map<storage_name, <update_period, storage_table>> in some places and I see map<update_peirod_storage_name, storage_table> in some places.
I feel it can degrade code readability; and whenever storage names have to be pulled, we need to understand where is actual storage names vs modified storage names. Can we be consistent in creating storage name as actual storage name alone, no other storage names - and move all the prefixing to final place of physical table name creation.
- Amareshwari Sriramadasu
On Jan. 19, 2017, 12:08 p.m., Lavkesh Lahngir wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/55712/
> -----------------------------------------------------------
>
> (Updated Jan. 19, 2017, 12:08 p.m.)
>
>
> Review request for lens.
>
>
> Bugs: LENS-1386
> https://issues.apache.org/jira/browse/LENS-1386
>
>
> Repository: lens
>
>
> Description
> -------
>
> A new data structure XUpdatePeriodTableDescriptor is introduced which contains an update period and table descriptor. Now the XUpdatePeriods will contain a list of XUpdatePeriodTableDescriptor or XUpdatePeriod
>
>
> Diffs
> -----
>
> lens-api/src/main/resources/cube-0.1.xsd f438f48
> lens-cube/src/main/java/org/apache/lens/cube/metadata/CubeFactTable.java adb6c92
> lens-cube/src/main/java/org/apache/lens/cube/metadata/CubeMetastoreClient.java 6c9cde2
> lens-cube/src/main/java/org/apache/lens/cube/metadata/MetastoreUtil.java 53cf8af
> lens-server/src/main/java/org/apache/lens/server/metastore/CubeMetastoreServiceImpl.java 8b10d1d
> lens-server/src/main/java/org/apache/lens/server/metastore/JAXBUtils.java 51fcb43
>
> Diff: https://reviews.apache.org/r/55712/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Lavkesh Lahngir
>
>
Re: Review Request 55712: Fact Schema change to support all update
periods in one storage
Posted by Amareshwari Sriramadasu <am...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/55712/#review162385
-----------------------------------------------------------
lens-cube/src/main/java/org/apache/lens/cube/metadata/CubeMetastoreClient.java (lines 225 - 226)
<https://reviews.apache.org/r/55712/#comment233735>
empty if?
lens-cube/src/main/java/org/apache/lens/cube/metadata/MetastoreUtil.java (line 594)
<https://reviews.apache.org/r/55712/#comment233736>
Should we call it updatePeriodTableKey?
lens-server/src/main/java/org/apache/lens/server/metastore/CubeMetastoreServiceImpl.java (lines 664 - 668)
<https://reviews.apache.org/r/55712/#comment233737>
Can addPartition in CubeMetastoreClient access updatePeriod from partition spec to find final storage table, instead of creating a temp storage name? That would be cleaner.
lens-server/src/main/java/org/apache/lens/server/metastore/CubeMetastoreServiceImpl.java (lines 685 - 697)
<https://reviews.apache.org/r/55712/#comment233738>
Same as above, can we move this cubemetastoreclient?
lens-server/src/main/java/org/apache/lens/server/metastore/JAXBUtils.java (line 867)
<https://reviews.apache.org/r/55712/#comment233739>
Changing storage name to updatePeriod_storage is looking more like a hack.
- Amareshwari Sriramadasu
On Jan. 19, 2017, 12:08 p.m., Lavkesh Lahngir wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/55712/
> -----------------------------------------------------------
>
> (Updated Jan. 19, 2017, 12:08 p.m.)
>
>
> Review request for lens.
>
>
> Bugs: LENS-1386
> https://issues.apache.org/jira/browse/LENS-1386
>
>
> Repository: lens
>
>
> Description
> -------
>
> A new data structure XUpdatePeriodTableDescriptor is introduced which contains an update period and table descriptor. Now the XUpdatePeriods will contain a list of XUpdatePeriodTableDescriptor or XUpdatePeriod
>
>
> Diffs
> -----
>
> lens-api/src/main/resources/cube-0.1.xsd f438f48
> lens-cube/src/main/java/org/apache/lens/cube/metadata/CubeFactTable.java adb6c92
> lens-cube/src/main/java/org/apache/lens/cube/metadata/CubeMetastoreClient.java 6c9cde2
> lens-cube/src/main/java/org/apache/lens/cube/metadata/MetastoreUtil.java 53cf8af
> lens-server/src/main/java/org/apache/lens/server/metastore/CubeMetastoreServiceImpl.java 8b10d1d
> lens-server/src/main/java/org/apache/lens/server/metastore/JAXBUtils.java 51fcb43
>
> Diff: https://reviews.apache.org/r/55712/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Lavkesh Lahngir
>
>
Re: Review Request 55712: Fact Schema change to support all update
periods in one storage
Posted by Amareshwari Sriramadasu <am...@apache.org>.
> On Jan. 24, 2017, 8:39 a.m., Amareshwari Sriramadasu wrote:
> > lens-cube/src/main/java/org/apache/lens/cube/metadata/CubeFactTable.java, line 111
> > <https://reviews.apache.org/r/55712/diff/2/?file=1612879#file1612879line111>
> >
> > Should this map be populated for Facts which are created before this feature as well?
Reopening this, as i dont see any changes associated
- Amareshwari
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/55712/#review162787
-----------------------------------------------------------
On Jan. 24, 2017, 2:57 p.m., Lavkesh Lahngir wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/55712/
> -----------------------------------------------------------
>
> (Updated Jan. 24, 2017, 2:57 p.m.)
>
>
> Review request for lens.
>
>
> Bugs: LENS-1386
> https://issues.apache.org/jira/browse/LENS-1386
>
>
> Repository: lens
>
>
> Description
> -------
>
> A new data structure XUpdatePeriodTableDescriptor is introduced which contains an update period and table descriptor. Now the XUpdatePeriods will contain a list of XUpdatePeriodTableDescriptor or XUpdatePeriod
>
>
> Diffs
> -----
>
> lens-api/src/main/resources/cube-0.1.xsd f438f48
> lens-cube/src/main/java/org/apache/lens/cube/metadata/CubeFactTable.java adb6c92
> lens-cube/src/main/java/org/apache/lens/cube/metadata/CubeMetastoreClient.java 6c9cde2
> lens-cube/src/main/java/org/apache/lens/cube/metadata/MetastoreUtil.java 53cf8af
> lens-cube/src/main/java/org/apache/lens/cube/metadata/Storage.java cd9f705
> lens-cube/src/test/java/org/apache/lens/cube/metadata/TestCubeMetastoreClient.java e21dc2a
> lens-server/src/main/java/org/apache/lens/server/metastore/CubeMetastoreServiceImpl.java 8b10d1d
> lens-server/src/main/java/org/apache/lens/server/metastore/JAXBUtils.java 51fcb43
>
> Diff: https://reviews.apache.org/r/55712/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Lavkesh Lahngir
>
>
Re: Review Request 55712: Fact Schema change to support all update
periods in one storage
Posted by Lavkesh Lahngir <la...@gmail.com>.
> On Jan. 24, 2017, 8:39 a.m., Amareshwari Sriramadasu wrote:
> > lens-cube/src/main/java/org/apache/lens/cube/metadata/CubeFactTable.java, line 111
> > <https://reviews.apache.org/r/55712/diff/2/?file=1612879#file1612879line111>
> >
> > Should this map be populated for Facts which are created before this feature as well?
>
> Amareshwari Sriramadasu wrote:
> Reopening this, as i dont see any changes associated
We can use this map to get storage_table_prefix when the storage name and updateperiod is given.
- Lavkesh
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/55712/#review162787
-----------------------------------------------------------
On Jan. 24, 2017, 2:57 p.m., Lavkesh Lahngir wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/55712/
> -----------------------------------------------------------
>
> (Updated Jan. 24, 2017, 2:57 p.m.)
>
>
> Review request for lens.
>
>
> Bugs: LENS-1386
> https://issues.apache.org/jira/browse/LENS-1386
>
>
> Repository: lens
>
>
> Description
> -------
>
> A new data structure XUpdatePeriodTableDescriptor is introduced which contains an update period and table descriptor. Now the XUpdatePeriods will contain a list of XUpdatePeriodTableDescriptor or XUpdatePeriod
>
>
> Diffs
> -----
>
> lens-api/src/main/resources/cube-0.1.xsd f438f48
> lens-cube/src/main/java/org/apache/lens/cube/metadata/CubeFactTable.java adb6c92
> lens-cube/src/main/java/org/apache/lens/cube/metadata/CubeMetastoreClient.java 6c9cde2
> lens-cube/src/main/java/org/apache/lens/cube/metadata/MetastoreUtil.java 53cf8af
> lens-cube/src/main/java/org/apache/lens/cube/metadata/Storage.java cd9f705
> lens-cube/src/test/java/org/apache/lens/cube/metadata/TestCubeMetastoreClient.java e21dc2a
> lens-server/src/main/java/org/apache/lens/server/metastore/CubeMetastoreServiceImpl.java 8b10d1d
> lens-server/src/main/java/org/apache/lens/server/metastore/JAXBUtils.java 51fcb43
>
> Diff: https://reviews.apache.org/r/55712/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Lavkesh Lahngir
>
>
Re: Review Request 55712: Fact Schema change to support all update
periods in one storage
Posted by Amareshwari Sriramadasu <am...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/55712/#review162787
-----------------------------------------------------------
lens-cube/src/main/java/org/apache/lens/cube/metadata/CubeFactTable.java (line 37)
<https://reviews.apache.org/r/55712/#comment234139>
Please update the comment to state the value is table_name_prefix
lens-cube/src/main/java/org/apache/lens/cube/metadata/CubeFactTable.java (line 39)
<https://reviews.apache.org/r/55712/#comment234137>
Lets change the map to Map<String, Map<UpdatePeriod, String>>
lens-cube/src/main/java/org/apache/lens/cube/metadata/CubeFactTable.java (line 111)
<https://reviews.apache.org/r/55712/#comment234138>
Should this map be populated for Facts which are created before this feature as well?
lens-cube/src/main/java/org/apache/lens/cube/metadata/CubeMetastoreClient.java (line 189)
<https://reviews.apache.org/r/55712/#comment234141>
Is the method giving StorageTableName or storageTablePrefix ? Please name method or variable accordingly.
lens-cube/src/main/java/org/apache/lens/cube/metadata/CubeMetastoreClient.java (line 192)
<https://reviews.apache.org/r/55712/#comment234142>
Why is storageTablePrefix required for look up on partitionTimelineCache ?
what is passed for key?
lens-cube/src/main/java/org/apache/lens/cube/metadata/CubeMetastoreClient.java (lines 225 - 229)
<https://reviews.apache.org/r/55712/#comment234143>
Did not understand why these code changes are required.
lens-cube/src/main/java/org/apache/lens/cube/metadata/CubeMetastoreClient.java (line 345)
<https://reviews.apache.org/r/55712/#comment234144>
We need understand why storagePrefix is required for PartitionTimeline.
I feel it should not be required.
lens-cube/src/main/java/org/apache/lens/cube/metadata/CubeMetastoreClient.java (line 754)
<https://reviews.apache.org/r/55712/#comment234145>
Can you update the comment to what is it now?
lens-cube/src/main/java/org/apache/lens/cube/metadata/CubeMetastoreClient.java (line 973)
<https://reviews.apache.org/r/55712/#comment234146>
Why is getPrefix method taking storageTableName as parameter?
lens-cube/src/main/java/org/apache/lens/cube/metadata/CubeMetastoreClient.java (line 977)
<https://reviews.apache.org/r/55712/#comment234147>
Same as above, why is getPrefix taking storageTableName as prefix?
lens-cube/src/main/java/org/apache/lens/cube/metadata/CubeMetastoreClient.java (line 2275)
<https://reviews.apache.org/r/55712/#comment234149>
How will this work if is same table is mapped for all update periods?
lens-cube/src/main/java/org/apache/lens/cube/metadata/CubeMetastoreClient.java (line 2278)
<https://reviews.apache.org/r/55712/#comment234148>
I see drop is already happening before.
lens-cube/src/main/java/org/apache/lens/cube/metadata/CubeMetastoreClient.java (line 2490)
<https://reviews.apache.org/r/55712/#comment234150>
Please name the variable and method appropriately. And should this method be moved to CubeFactTable?
lens-cube/src/main/java/org/apache/lens/cube/metadata/Storage.java (line 133)
<https://reviews.apache.org/r/55712/#comment234153>
Should we name the param as storageTableNamePrefix? Also, please update javadoc accordingly.
lens-cube/src/main/java/org/apache/lens/cube/metadata/Storage.java (line 266)
<https://reviews.apache.org/r/55712/#comment234154>
Rename param to storageTableNamePrefix?
lens-cube/src/main/java/org/apache/lens/cube/metadata/Storage.java (line 387)
<https://reviews.apache.org/r/55712/#comment234155>
Rename param to storageTableNamePrefix?
lens-server/src/main/java/org/apache/lens/server/metastore/JAXBUtils.java (line 860)
<https://reviews.apache.org/r/55712/#comment234156>
Can we rename the map appropriately?
- Amareshwari Sriramadasu
On Jan. 23, 2017, 2:51 p.m., Lavkesh Lahngir wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/55712/
> -----------------------------------------------------------
>
> (Updated Jan. 23, 2017, 2:51 p.m.)
>
>
> Review request for lens.
>
>
> Bugs: LENS-1386
> https://issues.apache.org/jira/browse/LENS-1386
>
>
> Repository: lens
>
>
> Description
> -------
>
> A new data structure XUpdatePeriodTableDescriptor is introduced which contains an update period and table descriptor. Now the XUpdatePeriods will contain a list of XUpdatePeriodTableDescriptor or XUpdatePeriod
>
>
> Diffs
> -----
>
> lens-api/src/main/resources/cube-0.1.xsd f438f48
> lens-cube/src/main/java/org/apache/lens/cube/metadata/CubeFactTable.java adb6c92
> lens-cube/src/main/java/org/apache/lens/cube/metadata/CubeMetastoreClient.java 6c9cde2
> lens-cube/src/main/java/org/apache/lens/cube/metadata/MetastoreUtil.java 53cf8af
> lens-cube/src/main/java/org/apache/lens/cube/metadata/Storage.java cd9f705
> lens-cube/src/test/java/org/apache/lens/cube/metadata/TestCubeMetastoreClient.java e21dc2a
> lens-server/src/main/java/org/apache/lens/server/metastore/CubeMetastoreServiceImpl.java 8b10d1d
> lens-server/src/main/java/org/apache/lens/server/metastore/JAXBUtils.java 51fcb43
>
> Diff: https://reviews.apache.org/r/55712/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Lavkesh Lahngir
>
>
Re: Review Request 55712: Fact Schema change to support all update
periods in one storage
Posted by Amareshwari Sriramadasu <am...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/55712/#review164100
-----------------------------------------------------------
lens-api/src/main/resources/cube-0.1.xsd (line 1029)
<https://reviews.apache.org/r/55712/#comment235674>
Should we update doc here that if table descriptions are specified in x_update_periods, then this table description is not vaild?
lens-cube/src/main/java/org/apache/lens/cube/metadata/CubeFactTable.java (line 117)
<https://reviews.apache.org/r/55712/#comment235675>
rename this to storageTablePrefix?
lens-cube/src/test/java/org/apache/lens/cube/metadata/TestCubeMetastoreClient.java (line 1000)
<https://reviews.apache.org/r/55712/#comment235677>
uncomment assert?
lens-server/src/main/java/org/apache/lens/server/metastore/CubeMetastoreServiceImpl.java (line 396)
<https://reviews.apache.org/r/55712/#comment235678>
Add unit test in TestMetastoreService for CRUD
- Amareshwari Sriramadasu
On Feb. 3, 2017, 8:01 a.m., Lavkesh Lahngir wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/55712/
> -----------------------------------------------------------
>
> (Updated Feb. 3, 2017, 8:01 a.m.)
>
>
> Review request for lens.
>
>
> Bugs: LENS-1386
> https://issues.apache.org/jira/browse/LENS-1386
>
>
> Repository: lens
>
>
> Description
> -------
>
> A new data structure XUpdatePeriodTableDescriptor is introduced which contains an update period and table descriptor. Now the XUpdatePeriods will contain a list of XUpdatePeriodTableDescriptor or XUpdatePeriod
>
>
> Diffs
> -----
>
> lens-api/src/main/resources/cube-0.1.xsd f438f48
> lens-cube/src/main/java/org/apache/lens/cube/metadata/CubeFactTable.java adb6c92
> lens-cube/src/main/java/org/apache/lens/cube/metadata/CubeMetastoreClient.java 6c9cde2
> lens-cube/src/main/java/org/apache/lens/cube/metadata/MetastoreUtil.java 53cf8af
> lens-cube/src/main/java/org/apache/lens/cube/metadata/Storage.java cd9f705
> lens-cube/src/test/java/org/apache/lens/cube/metadata/TestCubeMetastoreClient.java e21dc2a
> lens-server/src/main/java/org/apache/lens/server/metastore/CubeMetastoreServiceImpl.java 8b10d1d
> lens-server/src/main/java/org/apache/lens/server/metastore/JAXBUtils.java 51fcb43
>
> Diff: https://reviews.apache.org/r/55712/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Lavkesh Lahngir
>
>
Re: Review Request 55712: Fact Schema change to support all update
periods in one storage
Posted by Lavkesh Lahngir <la...@gmail.com>.
> On Feb. 14, 2017, 11:53 a.m., Amareshwari Sriramadasu wrote:
> > lens-cube/src/main/java/org/apache/lens/cube/metadata/CubeMetastoreClient.java, line 384
> > <https://reviews.apache.org/r/55712/diff/7/?file=1632298#file1632298line384>
> >
> > If we are calling this for each updatePeriod separately even if the table is same for all updatePeriods, it can have huge impact on the time it takes to create timelines and requires huge memory to put all partitions.
I will add a condition
if (!uniqueStorageTables.contains(storageTableName)) then only fetch partitions.
> On Feb. 14, 2017, 11:53 a.m., Amareshwari Sriramadasu wrote:
> > lens-cube/src/main/java/org/apache/lens/cube/metadata/CubeMetastoreClient.java, line 499
> > <https://reviews.apache.org/r/55712/diff/7/?file=1632298#file1632298line499>
> >
> > no more required? what was it doing earlier which is not required now?
The method code is merged with loadTimeLines()
> On Feb. 14, 2017, 11:53 a.m., Amareshwari Sriramadasu wrote:
> > lens-cube/src/main/java/org/apache/lens/cube/metadata/CubeMetastoreClient.java, line 948
> > <https://reviews.apache.org/r/55712/diff/7/?file=1632298#file1632298line948>
> >
> > Seems javadoc is wrong
Yeah.. correcting it.
- Lavkesh
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/55712/#review165474
-----------------------------------------------------------
On Feb. 13, 2017, 6:47 p.m., Lavkesh Lahngir wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/55712/
> -----------------------------------------------------------
>
> (Updated Feb. 13, 2017, 6:47 p.m.)
>
>
> Review request for lens.
>
>
> Bugs: LENS-1386
> https://issues.apache.org/jira/browse/LENS-1386
>
>
> Repository: lens
>
>
> Description
> -------
>
> A new data structure XUpdatePeriodTableDescriptor is introduced which contains an update period and table descriptor. Now the XUpdatePeriods will contain a list of XUpdatePeriodTableDescriptor or XUpdatePeriod
>
>
> Diffs
> -----
>
> lens-api/src/main/resources/cube-0.1.xsd f438f48
> lens-cube/src/main/java/org/apache/lens/cube/metadata/CubeFactTable.java adb6c92
> lens-cube/src/main/java/org/apache/lens/cube/metadata/CubeMetastoreClient.java 6c9cde2
> lens-cube/src/main/java/org/apache/lens/cube/metadata/MetastoreUtil.java 53cf8af
> lens-cube/src/main/java/org/apache/lens/cube/metadata/Storage.java cd9f705
> lens-cube/src/test/java/org/apache/lens/cube/metadata/TestCubeMetastoreClient.java e21dc2a
> lens-server/src/main/java/org/apache/lens/server/metastore/CubeMetastoreServiceImpl.java 8b10d1d
> lens-server/src/main/java/org/apache/lens/server/metastore/JAXBUtils.java 51fcb43
> lens-server/src/test/java/org/apache/lens/server/metastore/TestMetastoreService.java 0e6a4a1
>
> Diff: https://reviews.apache.org/r/55712/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Lavkesh Lahngir
>
>
Re: Review Request 55712: Fact Schema change to support all update
periods in one storage
Posted by Amareshwari Sriramadasu <am...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/55712/#review165474
-----------------------------------------------------------
lens-cube/src/main/java/org/apache/lens/cube/metadata/CubeMetastoreClient.java (line 383)
<https://reviews.apache.org/r/55712/#comment237325>
If we are calling this for each updatePeriod separately even if the table is same for all updatePeriods, it can have huge impact on the time it takes to create timelines and requires huge memory to put all partitions.
lens-cube/src/main/java/org/apache/lens/cube/metadata/CubeMetastoreClient.java
<https://reviews.apache.org/r/55712/#comment237326>
no more required? what was it doing earlier which is not required now?
lens-cube/src/main/java/org/apache/lens/cube/metadata/CubeMetastoreClient.java (line 913)
<https://reviews.apache.org/r/55712/#comment237327>
Seems javadoc is wrong
- Amareshwari Sriramadasu
On Feb. 13, 2017, 6:47 p.m., Lavkesh Lahngir wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/55712/
> -----------------------------------------------------------
>
> (Updated Feb. 13, 2017, 6:47 p.m.)
>
>
> Review request for lens.
>
>
> Bugs: LENS-1386
> https://issues.apache.org/jira/browse/LENS-1386
>
>
> Repository: lens
>
>
> Description
> -------
>
> A new data structure XUpdatePeriodTableDescriptor is introduced which contains an update period and table descriptor. Now the XUpdatePeriods will contain a list of XUpdatePeriodTableDescriptor or XUpdatePeriod
>
>
> Diffs
> -----
>
> lens-api/src/main/resources/cube-0.1.xsd f438f48
> lens-cube/src/main/java/org/apache/lens/cube/metadata/CubeFactTable.java adb6c92
> lens-cube/src/main/java/org/apache/lens/cube/metadata/CubeMetastoreClient.java 6c9cde2
> lens-cube/src/main/java/org/apache/lens/cube/metadata/MetastoreUtil.java 53cf8af
> lens-cube/src/main/java/org/apache/lens/cube/metadata/Storage.java cd9f705
> lens-cube/src/test/java/org/apache/lens/cube/metadata/TestCubeMetastoreClient.java e21dc2a
> lens-server/src/main/java/org/apache/lens/server/metastore/CubeMetastoreServiceImpl.java 8b10d1d
> lens-server/src/main/java/org/apache/lens/server/metastore/JAXBUtils.java 51fcb43
> lens-server/src/test/java/org/apache/lens/server/metastore/TestMetastoreService.java 0e6a4a1
>
> Diff: https://reviews.apache.org/r/55712/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Lavkesh Lahngir
>
>
Re: Review Request 55712: Fact Schema change to support all update
periods in one storage
Posted by Amareshwari Sriramadasu <am...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/55712/#review166063
-----------------------------------------------------------
Ship it!
Ship It!
- Amareshwari Sriramadasu
On Feb. 19, 2017, 4:26 a.m., Lavkesh Lahngir wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/55712/
> -----------------------------------------------------------
>
> (Updated Feb. 19, 2017, 4:26 a.m.)
>
>
> Review request for lens.
>
>
> Bugs: LENS-1386
> https://issues.apache.org/jira/browse/LENS-1386
>
>
> Repository: lens
>
>
> Description
> -------
>
> A new data structure XUpdatePeriodTableDescriptor is introduced which contains an update period and table descriptor. Now the XUpdatePeriods will contain a list of XUpdatePeriodTableDescriptor or XUpdatePeriod
>
>
> Diffs
> -----
>
> lens-api/src/main/resources/cube-0.1.xsd f438f48
> lens-cube/src/main/java/org/apache/lens/cube/metadata/CubeFactTable.java adb6c92
> lens-cube/src/main/java/org/apache/lens/cube/metadata/CubeMetastoreClient.java 6c9cde2
> lens-cube/src/main/java/org/apache/lens/cube/metadata/MetastoreUtil.java 53cf8af
> lens-cube/src/main/java/org/apache/lens/cube/metadata/Storage.java cd9f705
> lens-cube/src/test/java/org/apache/lens/cube/metadata/TestCubeMetastoreClient.java e21dc2a
> lens-server/src/main/java/org/apache/lens/server/metastore/CubeMetastoreServiceImpl.java 8b10d1d
> lens-server/src/main/java/org/apache/lens/server/metastore/JAXBUtils.java 51fcb43
> lens-server/src/test/java/org/apache/lens/server/metastore/TestMetastoreService.java 0e6a4a1
>
> Diff: https://reviews.apache.org/r/55712/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Lavkesh Lahngir
>
>
Re: Review Request 55712: Fact Schema change to support all update
periods in one storage
Posted by Lavkesh Lahngir <la...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/55712/
-----------------------------------------------------------
(Updated Feb. 19, 2017, 4:26 a.m.)
Review request for lens.
Changes
-------
Final version with test cases fixes and review comments
Bugs: LENS-1386
https://issues.apache.org/jira/browse/LENS-1386
Repository: lens
Description
-------
A new data structure XUpdatePeriodTableDescriptor is introduced which contains an update period and table descriptor. Now the XUpdatePeriods will contain a list of XUpdatePeriodTableDescriptor or XUpdatePeriod
Diffs (updated)
-----
lens-api/src/main/resources/cube-0.1.xsd f438f48
lens-cube/src/main/java/org/apache/lens/cube/metadata/CubeFactTable.java adb6c92
lens-cube/src/main/java/org/apache/lens/cube/metadata/CubeMetastoreClient.java 6c9cde2
lens-cube/src/main/java/org/apache/lens/cube/metadata/MetastoreUtil.java 53cf8af
lens-cube/src/main/java/org/apache/lens/cube/metadata/Storage.java cd9f705
lens-cube/src/test/java/org/apache/lens/cube/metadata/TestCubeMetastoreClient.java e21dc2a
lens-server/src/main/java/org/apache/lens/server/metastore/CubeMetastoreServiceImpl.java 8b10d1d
lens-server/src/main/java/org/apache/lens/server/metastore/JAXBUtils.java 51fcb43
lens-server/src/test/java/org/apache/lens/server/metastore/TestMetastoreService.java 0e6a4a1
Diff: https://reviews.apache.org/r/55712/diff/
Testing
-------
Thanks,
Lavkesh Lahngir
Re: Review Request 55712: Fact Schema change to support all update
periods in one storage
Posted by Lavkesh Lahngir <la...@gmail.com>.
> On Feb. 15, 2017, 4:17 a.m., Amareshwari Sriramadasu wrote:
> > lens-cube/src/main/java/org/apache/lens/cube/metadata/CubeMetastoreClient.java, line 2528
> > <https://reviews.apache.org/r/55712/diff/7/?file=1632298#file1632298line2528>
> >
> > Please avoid using exceptions in normal flow. Why would getFactTable().getTablePrefix() throw exception?
In the addpartitions() flow, the parent table can be a dimension which means the getFactTable() call will throw an exception.
> On Feb. 15, 2017, 4:17 a.m., Amareshwari Sriramadasu wrote:
> > lens-server/src/main/java/org/apache/lens/server/metastore/CubeMetastoreServiceImpl.java, line 653
> > <https://reviews.apache.org/r/55712/diff/7/?file=1632302#file1632302line653>
> >
> > Please avoid using exception in normal flow.
This is added because of checkFactStorage() method. Because you could get dimention table also.
> On Feb. 15, 2017, 4:17 a.m., Amareshwari Sriramadasu wrote:
> > lens-server/src/test/java/org/apache/lens/server/metastore/TestMetastoreService.java, line 1877
> > <https://reviews.apache.org/r/55712/diff/7/?file=1632304#file1632304line1877>
> >
> > What are these table names are for?
These are set inside Storagetabledesc's location path.
- Lavkesh
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/55712/#review165641
-----------------------------------------------------------
On Feb. 13, 2017, 6:47 p.m., Lavkesh Lahngir wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/55712/
> -----------------------------------------------------------
>
> (Updated Feb. 13, 2017, 6:47 p.m.)
>
>
> Review request for lens.
>
>
> Bugs: LENS-1386
> https://issues.apache.org/jira/browse/LENS-1386
>
>
> Repository: lens
>
>
> Description
> -------
>
> A new data structure XUpdatePeriodTableDescriptor is introduced which contains an update period and table descriptor. Now the XUpdatePeriods will contain a list of XUpdatePeriodTableDescriptor or XUpdatePeriod
>
>
> Diffs
> -----
>
> lens-api/src/main/resources/cube-0.1.xsd f438f48
> lens-cube/src/main/java/org/apache/lens/cube/metadata/CubeFactTable.java adb6c92
> lens-cube/src/main/java/org/apache/lens/cube/metadata/CubeMetastoreClient.java 6c9cde2
> lens-cube/src/main/java/org/apache/lens/cube/metadata/MetastoreUtil.java 53cf8af
> lens-cube/src/main/java/org/apache/lens/cube/metadata/Storage.java cd9f705
> lens-cube/src/test/java/org/apache/lens/cube/metadata/TestCubeMetastoreClient.java e21dc2a
> lens-server/src/main/java/org/apache/lens/server/metastore/CubeMetastoreServiceImpl.java 8b10d1d
> lens-server/src/main/java/org/apache/lens/server/metastore/JAXBUtils.java 51fcb43
> lens-server/src/test/java/org/apache/lens/server/metastore/TestMetastoreService.java 0e6a4a1
>
> Diff: https://reviews.apache.org/r/55712/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Lavkesh Lahngir
>
>
Re: Review Request 55712: Fact Schema change to support all update
periods in one storage
Posted by Amareshwari Sriramadasu <am...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/55712/#review165641
-----------------------------------------------------------
lens-cube/src/main/java/org/apache/lens/cube/metadata/CubeMetastoreClient.java (line 1064)
<https://reviews.apache.org/r/55712/#comment237505>
We have to iterate over update periods that belong to the storageTableName only. Not all updatePeriods.
lens-cube/src/main/java/org/apache/lens/cube/metadata/CubeMetastoreClient.java (line 2491)
<https://reviews.apache.org/r/55712/#comment237506>
Please avoid using exceptions in normal flow. Why would getFactTable().getTablePrefix() throw exception?
lens-cube/src/test/java/org/apache/lens/cube/metadata/TestCubeMetastoreClient.java (line 973)
<https://reviews.apache.org/r/55712/#comment237507>
Can you add partitions to these tables and validate partition timelines?
lens-server/src/main/java/org/apache/lens/server/metastore/CubeMetastoreServiceImpl.java (line 653)
<https://reviews.apache.org/r/55712/#comment237508>
Please avoid using exception in normal flow.
lens-server/src/main/java/org/apache/lens/server/metastore/CubeMetastoreServiceImpl.java (line 749)
<https://reviews.apache.org/r/55712/#comment237509>
Same as above, avoid exceptions in normal flow.
lens-server/src/test/java/org/apache/lens/server/metastore/TestMetastoreService.java (line 1874)
<https://reviews.apache.org/r/55712/#comment237511>
What are these table names are for?
lens-server/src/test/java/org/apache/lens/server/metastore/TestMetastoreService.java (line 1886)
<https://reviews.apache.org/r/55712/#comment237512>
Can you pull native tables and validate the separate tables for each update period exist?
lens-server/src/test/java/org/apache/lens/server/metastore/TestMetastoreService.java (line 2003)
<https://reviews.apache.org/r/55712/#comment237510>
Can you drop fact and assert all storage tables are dropped? You might need to pull native tables to check the same
- Amareshwari Sriramadasu
On Feb. 13, 2017, 6:47 p.m., Lavkesh Lahngir wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/55712/
> -----------------------------------------------------------
>
> (Updated Feb. 13, 2017, 6:47 p.m.)
>
>
> Review request for lens.
>
>
> Bugs: LENS-1386
> https://issues.apache.org/jira/browse/LENS-1386
>
>
> Repository: lens
>
>
> Description
> -------
>
> A new data structure XUpdatePeriodTableDescriptor is introduced which contains an update period and table descriptor. Now the XUpdatePeriods will contain a list of XUpdatePeriodTableDescriptor or XUpdatePeriod
>
>
> Diffs
> -----
>
> lens-api/src/main/resources/cube-0.1.xsd f438f48
> lens-cube/src/main/java/org/apache/lens/cube/metadata/CubeFactTable.java adb6c92
> lens-cube/src/main/java/org/apache/lens/cube/metadata/CubeMetastoreClient.java 6c9cde2
> lens-cube/src/main/java/org/apache/lens/cube/metadata/MetastoreUtil.java 53cf8af
> lens-cube/src/main/java/org/apache/lens/cube/metadata/Storage.java cd9f705
> lens-cube/src/test/java/org/apache/lens/cube/metadata/TestCubeMetastoreClient.java e21dc2a
> lens-server/src/main/java/org/apache/lens/server/metastore/CubeMetastoreServiceImpl.java 8b10d1d
> lens-server/src/main/java/org/apache/lens/server/metastore/JAXBUtils.java 51fcb43
> lens-server/src/test/java/org/apache/lens/server/metastore/TestMetastoreService.java 0e6a4a1
>
> Diff: https://reviews.apache.org/r/55712/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Lavkesh Lahngir
>
>
Re: Review Request 55712: Fact Schema change to support all update
periods in one storage
Posted by Lavkesh Lahngir <la...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/55712/
-----------------------------------------------------------
(Updated Feb. 13, 2017, 6:47 p.m.)
Review request for lens.
Changes
-------
Test cases added
Bugs: LENS-1386
https://issues.apache.org/jira/browse/LENS-1386
Repository: lens
Description
-------
A new data structure XUpdatePeriodTableDescriptor is introduced which contains an update period and table descriptor. Now the XUpdatePeriods will contain a list of XUpdatePeriodTableDescriptor or XUpdatePeriod
Diffs (updated)
-----
lens-api/src/main/resources/cube-0.1.xsd f438f48
lens-cube/src/main/java/org/apache/lens/cube/metadata/CubeFactTable.java adb6c92
lens-cube/src/main/java/org/apache/lens/cube/metadata/CubeMetastoreClient.java 6c9cde2
lens-cube/src/main/java/org/apache/lens/cube/metadata/MetastoreUtil.java 53cf8af
lens-cube/src/main/java/org/apache/lens/cube/metadata/Storage.java cd9f705
lens-cube/src/test/java/org/apache/lens/cube/metadata/TestCubeMetastoreClient.java e21dc2a
lens-server/src/main/java/org/apache/lens/server/metastore/CubeMetastoreServiceImpl.java 8b10d1d
lens-server/src/main/java/org/apache/lens/server/metastore/JAXBUtils.java 51fcb43
lens-server/src/test/java/org/apache/lens/server/metastore/TestMetastoreService.java 0e6a4a1
Diff: https://reviews.apache.org/r/55712/diff/
Testing
-------
Thanks,
Lavkesh Lahngir
Re: Review Request 55712: Fact Schema change to support all update
periods in one storage
Posted by Lavkesh Lahngir <la...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/55712/
-----------------------------------------------------------
(Updated Feb. 4, 2017, 12:03 a.m.)
Review request for lens.
Changes
-------
Checkstyle changes
Bugs: LENS-1386
https://issues.apache.org/jira/browse/LENS-1386
Repository: lens
Description
-------
A new data structure XUpdatePeriodTableDescriptor is introduced which contains an update period and table descriptor. Now the XUpdatePeriods will contain a list of XUpdatePeriodTableDescriptor or XUpdatePeriod
Diffs (updated)
-----
lens-api/src/main/resources/cube-0.1.xsd f438f48
lens-cube/src/main/java/org/apache/lens/cube/metadata/CubeFactTable.java adb6c92
lens-cube/src/main/java/org/apache/lens/cube/metadata/CubeMetastoreClient.java 6c9cde2
lens-cube/src/main/java/org/apache/lens/cube/metadata/MetastoreUtil.java 53cf8af
lens-cube/src/main/java/org/apache/lens/cube/metadata/Storage.java cd9f705
lens-cube/src/test/java/org/apache/lens/cube/metadata/TestCubeMetastoreClient.java e21dc2a
lens-server/src/main/java/org/apache/lens/server/metastore/CubeMetastoreServiceImpl.java 8b10d1d
lens-server/src/main/java/org/apache/lens/server/metastore/JAXBUtils.java 51fcb43
Diff: https://reviews.apache.org/r/55712/diff/
Testing
-------
Thanks,
Lavkesh Lahngir
Re: Review Request 55712: Fact Schema change to support all update
periods in one storage
Posted by Lavkesh Lahngir <la...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/55712/
-----------------------------------------------------------
(Updated Feb. 3, 2017, 8:01 a.m.)
Review request for lens.
Changes
-------
Review comments
Bugs: LENS-1386
https://issues.apache.org/jira/browse/LENS-1386
Repository: lens
Description
-------
A new data structure XUpdatePeriodTableDescriptor is introduced which contains an update period and table descriptor. Now the XUpdatePeriods will contain a list of XUpdatePeriodTableDescriptor or XUpdatePeriod
Diffs (updated)
-----
lens-api/src/main/resources/cube-0.1.xsd f438f48
lens-cube/src/main/java/org/apache/lens/cube/metadata/CubeFactTable.java adb6c92
lens-cube/src/main/java/org/apache/lens/cube/metadata/CubeMetastoreClient.java 6c9cde2
lens-cube/src/main/java/org/apache/lens/cube/metadata/MetastoreUtil.java 53cf8af
lens-cube/src/main/java/org/apache/lens/cube/metadata/Storage.java cd9f705
lens-cube/src/test/java/org/apache/lens/cube/metadata/TestCubeMetastoreClient.java e21dc2a
lens-server/src/main/java/org/apache/lens/server/metastore/CubeMetastoreServiceImpl.java 8b10d1d
lens-server/src/main/java/org/apache/lens/server/metastore/JAXBUtils.java 51fcb43
Diff: https://reviews.apache.org/r/55712/diff/
Testing
-------
Thanks,
Lavkesh Lahngir
Re: Review Request 55712: Fact Schema change to support all update
periods in one storage
Posted by Amareshwari Sriramadasu <am...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/55712/#review163785
-----------------------------------------------------------
lens-cube/src/main/java/org/apache/lens/cube/metadata/CubeMetastoreClient.java (line 190)
<https://reviews.apache.org/r/55712/#comment235283>
Seems variable is not used, can be removed?
lens-cube/src/main/java/org/apache/lens/cube/metadata/CubeMetastoreClient.java (line 347)
<https://reviews.apache.org/r/55712/#comment235284>
Why is timeLineKey storageTableName? And tableName is only constructed with fact and storage names?
- Amareshwari Sriramadasu
On Jan. 31, 2017, 1:53 p.m., Lavkesh Lahngir wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/55712/
> -----------------------------------------------------------
>
> (Updated Jan. 31, 2017, 1:53 p.m.)
>
>
> Review request for lens.
>
>
> Bugs: LENS-1386
> https://issues.apache.org/jira/browse/LENS-1386
>
>
> Repository: lens
>
>
> Description
> -------
>
> A new data structure XUpdatePeriodTableDescriptor is introduced which contains an update period and table descriptor. Now the XUpdatePeriods will contain a list of XUpdatePeriodTableDescriptor or XUpdatePeriod
>
>
> Diffs
> -----
>
> lens-api/src/main/resources/cube-0.1.xsd f438f48
> lens-cube/src/main/java/org/apache/lens/cube/metadata/CubeFactTable.java adb6c92
> lens-cube/src/main/java/org/apache/lens/cube/metadata/CubeMetastoreClient.java 6c9cde2
> lens-cube/src/main/java/org/apache/lens/cube/metadata/MetastoreUtil.java 53cf8af
> lens-cube/src/main/java/org/apache/lens/cube/metadata/Storage.java cd9f705
> lens-cube/src/test/java/org/apache/lens/cube/metadata/TestCubeMetastoreClient.java e21dc2a
> lens-server/src/main/java/org/apache/lens/server/metastore/CubeMetastoreServiceImpl.java 8b10d1d
> lens-server/src/main/java/org/apache/lens/server/metastore/JAXBUtils.java 51fcb43
>
> Diff: https://reviews.apache.org/r/55712/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Lavkesh Lahngir
>
>
Re: Review Request 55712: Fact Schema change to support all update
periods in one storage
Posted by Amareshwari Sriramadasu <am...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/55712/#review163934
-----------------------------------------------------------
lens-api/src/main/resources/cube-0.1.xsd (line 684)
<https://reviews.apache.org/r/55712/#comment235458>
Can we add documentation describing the type?
lens-api/src/main/resources/cube-0.1.xsd (line 690)
<https://reviews.apache.org/r/55712/#comment235459>
Can you add documentation describing the type?
lens-cube/src/main/java/org/apache/lens/cube/metadata/CubeFactTable.java (lines 112 - 120)
<https://reviews.apache.org/r/55712/#comment235460>
Instead of reading properties and can we iterate over storageUpdatePeriods map?
lens-cube/src/main/java/org/apache/lens/cube/metadata/CubeMetastoreClient.java (line 2508)
<https://reviews.apache.org/r/55712/#comment235461>
instead of reading from property, can we read it from the map?
lens-cube/src/main/java/org/apache/lens/cube/metadata/CubeMetastoreClient.java (line 2510)
<https://reviews.apache.org/r/55712/#comment235462>
Seems wrong prefix here.
lens-cube/src/main/java/org/apache/lens/cube/metadata/CubeMetastoreClient.java (line 2512)
<https://reviews.apache.org/r/55712/#comment235463>
why is catch and ignore required?
- Amareshwari Sriramadasu
On Jan. 31, 2017, 1:53 p.m., Lavkesh Lahngir wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/55712/
> -----------------------------------------------------------
>
> (Updated Jan. 31, 2017, 1:53 p.m.)
>
>
> Review request for lens.
>
>
> Bugs: LENS-1386
> https://issues.apache.org/jira/browse/LENS-1386
>
>
> Repository: lens
>
>
> Description
> -------
>
> A new data structure XUpdatePeriodTableDescriptor is introduced which contains an update period and table descriptor. Now the XUpdatePeriods will contain a list of XUpdatePeriodTableDescriptor or XUpdatePeriod
>
>
> Diffs
> -----
>
> lens-api/src/main/resources/cube-0.1.xsd f438f48
> lens-cube/src/main/java/org/apache/lens/cube/metadata/CubeFactTable.java adb6c92
> lens-cube/src/main/java/org/apache/lens/cube/metadata/CubeMetastoreClient.java 6c9cde2
> lens-cube/src/main/java/org/apache/lens/cube/metadata/MetastoreUtil.java 53cf8af
> lens-cube/src/main/java/org/apache/lens/cube/metadata/Storage.java cd9f705
> lens-cube/src/test/java/org/apache/lens/cube/metadata/TestCubeMetastoreClient.java e21dc2a
> lens-server/src/main/java/org/apache/lens/server/metastore/CubeMetastoreServiceImpl.java 8b10d1d
> lens-server/src/main/java/org/apache/lens/server/metastore/JAXBUtils.java 51fcb43
>
> Diff: https://reviews.apache.org/r/55712/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Lavkesh Lahngir
>
>
Re: Review Request 55712: Fact Schema change to support all update
periods in one storage
Posted by Lavkesh Lahngir <la...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/55712/
-----------------------------------------------------------
(Updated Jan. 31, 2017, 1:53 p.m.)
Review request for lens.
Changes
-------
Updated TimeLineCache and addded test case
Bugs: LENS-1386
https://issues.apache.org/jira/browse/LENS-1386
Repository: lens
Description
-------
A new data structure XUpdatePeriodTableDescriptor is introduced which contains an update period and table descriptor. Now the XUpdatePeriods will contain a list of XUpdatePeriodTableDescriptor or XUpdatePeriod
Diffs (updated)
-----
lens-api/src/main/resources/cube-0.1.xsd f438f48
lens-cube/src/main/java/org/apache/lens/cube/metadata/CubeFactTable.java adb6c92
lens-cube/src/main/java/org/apache/lens/cube/metadata/CubeMetastoreClient.java 6c9cde2
lens-cube/src/main/java/org/apache/lens/cube/metadata/MetastoreUtil.java 53cf8af
lens-cube/src/main/java/org/apache/lens/cube/metadata/Storage.java cd9f705
lens-cube/src/test/java/org/apache/lens/cube/metadata/TestCubeMetastoreClient.java e21dc2a
lens-server/src/main/java/org/apache/lens/server/metastore/CubeMetastoreServiceImpl.java 8b10d1d
lens-server/src/main/java/org/apache/lens/server/metastore/JAXBUtils.java 51fcb43
Diff: https://reviews.apache.org/r/55712/diff/
Testing
-------
Thanks,
Lavkesh Lahngir
Re: Review Request 55712: Fact Schema change to support all update
periods in one storage
Posted by Lavkesh Lahngir <la...@gmail.com>.
> On Jan. 25, 2017, 8:16 a.m., Amareshwari Sriramadasu wrote:
> > lens-cube/src/main/java/org/apache/lens/cube/metadata/CubeMetastoreClient.java, line 344
> > <https://reviews.apache.org/r/55712/diff/3/?file=1613661#file1613661line344>
> >
> > If the method is returning Map of UpdatePeriod to PartitionTimeLine, I think updatePeriod should not passed as param.
> >
> > The method should populate the timeline for all updatePeriod, if required by pulling from different tables corresponding to each updatePeriod.
Then we should pass the table name prefix which is the key of PartitionTimelineCache.
> On Jan. 25, 2017, 8:16 a.m., Amareshwari Sriramadasu wrote:
> > lens-cube/src/main/java/org/apache/lens/cube/metadata/Storage.java, line 245
> > <https://reviews.apache.org/r/55712/diff/3/?file=1613663#file1613663line245>
> >
> > why commented?
It's not being used.
> On Jan. 25, 2017, 8:16 a.m., Amareshwari Sriramadasu wrote:
> > lens-cube/src/main/java/org/apache/lens/cube/metadata/CubeMetastoreClient.java, line 191
> > <https://reviews.apache.org/r/55712/diff/3/?file=1613661#file1613661line191>
> >
> > Seems updatePeriod can be null here. looking at the following check says so.
If the update period is null then the storage table name prefix will be the same as storage name.
- Lavkesh
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/55712/#review162925
-----------------------------------------------------------
On Jan. 24, 2017, 2:57 p.m., Lavkesh Lahngir wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/55712/
> -----------------------------------------------------------
>
> (Updated Jan. 24, 2017, 2:57 p.m.)
>
>
> Review request for lens.
>
>
> Bugs: LENS-1386
> https://issues.apache.org/jira/browse/LENS-1386
>
>
> Repository: lens
>
>
> Description
> -------
>
> A new data structure XUpdatePeriodTableDescriptor is introduced which contains an update period and table descriptor. Now the XUpdatePeriods will contain a list of XUpdatePeriodTableDescriptor or XUpdatePeriod
>
>
> Diffs
> -----
>
> lens-api/src/main/resources/cube-0.1.xsd f438f48
> lens-cube/src/main/java/org/apache/lens/cube/metadata/CubeFactTable.java adb6c92
> lens-cube/src/main/java/org/apache/lens/cube/metadata/CubeMetastoreClient.java 6c9cde2
> lens-cube/src/main/java/org/apache/lens/cube/metadata/MetastoreUtil.java 53cf8af
> lens-cube/src/main/java/org/apache/lens/cube/metadata/Storage.java cd9f705
> lens-cube/src/test/java/org/apache/lens/cube/metadata/TestCubeMetastoreClient.java e21dc2a
> lens-server/src/main/java/org/apache/lens/server/metastore/CubeMetastoreServiceImpl.java 8b10d1d
> lens-server/src/main/java/org/apache/lens/server/metastore/JAXBUtils.java 51fcb43
>
> Diff: https://reviews.apache.org/r/55712/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Lavkesh Lahngir
>
>
Re: Review Request 55712: Fact Schema change to support all update
periods in one storage
Posted by Amareshwari Sriramadasu <am...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/55712/#review162925
-----------------------------------------------------------
Please include unit tests
lens-cube/src/main/java/org/apache/lens/cube/metadata/CubeMetastoreClient.java (line 191)
<https://reviews.apache.org/r/55712/#comment234320>
Seems updatePeriod can be null here. looking at the following check says so.
lens-cube/src/main/java/org/apache/lens/cube/metadata/CubeMetastoreClient.java (line 343)
<https://reviews.apache.org/r/55712/#comment234321>
If the method is returning Map of UpdatePeriod to PartitionTimeLine, I think updatePeriod should not passed as param.
The method should populate the timeline for all updatePeriod, if required by pulling from different tables corresponding to each updatePeriod.
lens-cube/src/main/java/org/apache/lens/cube/metadata/CubeMetastoreClient.java (lines 379 - 383)
<https://reviews.apache.org/r/55712/#comment234322>
Looking at the code, similar thing should be done for get() as well.
lens-cube/src/main/java/org/apache/lens/cube/metadata/CubeMetastoreClient.java (line 865)
<https://reviews.apache.org/r/55712/#comment234323>
new param needs to be updated
lens-cube/src/main/java/org/apache/lens/cube/metadata/Storage.java (line 245)
<https://reviews.apache.org/r/55712/#comment234324>
why commented?
- Amareshwari Sriramadasu
On Jan. 24, 2017, 2:57 p.m., Lavkesh Lahngir wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/55712/
> -----------------------------------------------------------
>
> (Updated Jan. 24, 2017, 2:57 p.m.)
>
>
> Review request for lens.
>
>
> Bugs: LENS-1386
> https://issues.apache.org/jira/browse/LENS-1386
>
>
> Repository: lens
>
>
> Description
> -------
>
> A new data structure XUpdatePeriodTableDescriptor is introduced which contains an update period and table descriptor. Now the XUpdatePeriods will contain a list of XUpdatePeriodTableDescriptor or XUpdatePeriod
>
>
> Diffs
> -----
>
> lens-api/src/main/resources/cube-0.1.xsd f438f48
> lens-cube/src/main/java/org/apache/lens/cube/metadata/CubeFactTable.java adb6c92
> lens-cube/src/main/java/org/apache/lens/cube/metadata/CubeMetastoreClient.java 6c9cde2
> lens-cube/src/main/java/org/apache/lens/cube/metadata/MetastoreUtil.java 53cf8af
> lens-cube/src/main/java/org/apache/lens/cube/metadata/Storage.java cd9f705
> lens-cube/src/test/java/org/apache/lens/cube/metadata/TestCubeMetastoreClient.java e21dc2a
> lens-server/src/main/java/org/apache/lens/server/metastore/CubeMetastoreServiceImpl.java 8b10d1d
> lens-server/src/main/java/org/apache/lens/server/metastore/JAXBUtils.java 51fcb43
>
> Diff: https://reviews.apache.org/r/55712/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Lavkesh Lahngir
>
>
Re: Review Request 55712: Fact Schema change to support all update
periods in one storage
Posted by Lavkesh Lahngir <la...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/55712/
-----------------------------------------------------------
(Updated Jan. 24, 2017, 2:57 p.m.)
Review request for lens.
Bugs: LENS-1386
https://issues.apache.org/jira/browse/LENS-1386
Repository: lens
Description
-------
A new data structure XUpdatePeriodTableDescriptor is introduced which contains an update period and table descriptor. Now the XUpdatePeriods will contain a list of XUpdatePeriodTableDescriptor or XUpdatePeriod
Diffs (updated)
-----
lens-api/src/main/resources/cube-0.1.xsd f438f48
lens-cube/src/main/java/org/apache/lens/cube/metadata/CubeFactTable.java adb6c92
lens-cube/src/main/java/org/apache/lens/cube/metadata/CubeMetastoreClient.java 6c9cde2
lens-cube/src/main/java/org/apache/lens/cube/metadata/MetastoreUtil.java 53cf8af
lens-cube/src/main/java/org/apache/lens/cube/metadata/Storage.java cd9f705
lens-cube/src/test/java/org/apache/lens/cube/metadata/TestCubeMetastoreClient.java e21dc2a
lens-server/src/main/java/org/apache/lens/server/metastore/CubeMetastoreServiceImpl.java 8b10d1d
lens-server/src/main/java/org/apache/lens/server/metastore/JAXBUtils.java 51fcb43
Diff: https://reviews.apache.org/r/55712/diff/
Testing
-------
Thanks,
Lavkesh Lahngir
Re: Review Request 55712: Fact Schema change to support all update
periods in one storage
Posted by Lavkesh Lahngir <la...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/55712/
-----------------------------------------------------------
(Updated Jan. 23, 2017, 2:51 p.m.)
Review request for lens.
Bugs: LENS-1386
https://issues.apache.org/jira/browse/LENS-1386
Repository: lens
Description
-------
A new data structure XUpdatePeriodTableDescriptor is introduced which contains an update period and table descriptor. Now the XUpdatePeriods will contain a list of XUpdatePeriodTableDescriptor or XUpdatePeriod
Diffs (updated)
-----
lens-api/src/main/resources/cube-0.1.xsd f438f48
lens-cube/src/main/java/org/apache/lens/cube/metadata/CubeFactTable.java adb6c92
lens-cube/src/main/java/org/apache/lens/cube/metadata/CubeMetastoreClient.java 6c9cde2
lens-cube/src/main/java/org/apache/lens/cube/metadata/MetastoreUtil.java 53cf8af
lens-cube/src/main/java/org/apache/lens/cube/metadata/Storage.java cd9f705
lens-cube/src/test/java/org/apache/lens/cube/metadata/TestCubeMetastoreClient.java e21dc2a
lens-server/src/main/java/org/apache/lens/server/metastore/CubeMetastoreServiceImpl.java 8b10d1d
lens-server/src/main/java/org/apache/lens/server/metastore/JAXBUtils.java 51fcb43
Diff: https://reviews.apache.org/r/55712/diff/
Testing
-------
Thanks,
Lavkesh Lahngir