> On March 22, 2016, 4:41 a.m., Puneet Gupta wrote:
> > lens-api/src/main/java/org/apache/lens/api/TimeLines.java, line 71
> > <https://reviews.apache.org/r/45100/diff/1/?file=1308326#file1308326line71>
> >
> >     All implementations of PartitionTimeline do not support holes. Should 
> > this TimeLine be more verstalie (and not map each implemenatation to holes) 
> > and show the details based on which implementation of PartitionTimeline is 
> > being used ?
> 
> Ajay Yadava wrote:
>     All implementations do not contain holes as a separate field but we can 
> derive holes, that is exactly what we have done here.
> 
> Puneet Gupta wrote:
>     I meant, if a user is using RangesPartitionTimeline, he would like to see 
> the registered ranges. Similary for StoreAllPartitionTimeline, a user would 
> expect to see all the partitions and not start,end and holes. Not sure of 
> background of  LENS-819. Please ignore if the use case does not require this 
> behaviour.
> 
> Rajat Khandelwal wrote:
>     Any timeline implementation can be converted to any other timeline 
> implementation. Our goal is to return the exact timeline implementation to 
> the user. Each table has a choice of timeline implementation and when asked 
> about, the response should tell which implementation was being used. Not an 
> equivalent ends-and-holes timeline.
> 
> Ajay Yadava wrote:
>     I had an offline conversation with Amareshwari on both these issues to 
> understand the motivation. 
>     
>     1. It was specifically suggested to hide the internal implementation 
> side. 
>     2. Also, it was the explicit requirement to turn everything to ends and 
> holes representation as that is what made sense to the user and not the 
> internal implementations.
> 
> Rajat Khandelwal wrote:
>     I disagree. 
>     
>     
>     1. The "user" here is the schema designer. The decision of which timeline 
> implementation gets used stays with him. It's our job to show him the 
> timeline he chose on the REST API.
>     2. We don't get to make the desision on what makes sense to the user. 
> Again, falling back to 1 here, whatever he chose *does* make sense to him. 
> Drawing parallel to the rational donkey problem here, No implementation is 
> better/explicit than any other implementation. They all have trade-offs in 
> their access times. They all adhere to a common interface. But there's no 
> standard unit they should be converted to before sending out. They are 
> individual entities themselves. They are provided as options to the user to 
> choose from depending on their specific use case. 
>     3. The internal implementation that we should hide is the details of how 
> that gets stored on hive metastore (as table properties etc). Which we are 
> doing. None of the timeline subclasses are supposed to be "internal".
> 
> Ajay Yadava wrote:
>     I tend to agree with Amreshwari here, I believe it is more useful to see 
> the ends and holes of a partition rather than seeing the ranges or a huge 
> list of partitions and trying to figure out where are the holes. As an API 
> developer we have to make choices on behalf of users every day. Having said 
> that, I am open to alternatives and will wait for Amreshwari's thoughts as 
> well on this.
> 
> Amareshwari Sriramadasu wrote:
>     Let me put the motivation for adding a structure to the Timeline returned.
>     Giving a fixed structure gives the users to do programmatic decisions and 
> allow them to do complex automation without worrying about actual 
> implementation of timeline.
>     The current policy of returning the actual implementation makes them hard 
> to write any automation.
>     
>     If the schema designer is interested in actual implemenation and values 
> on server, there are always ways to see them directly on the store. With 
> current implementations, describe on native table gives all the info to the 
> designer.
>     
>     Does the above makes sense?
> 
> Rajat Khandelwal wrote:
>     A user can provide a custom implementation of timeline. I don't think it 
> makes sense to convert that to ends and holes. Secondly, about the structure, 
> I have a solution. We need to somehow return the map representation of the 
> timeline. Functions are already there and can be used. The conversion might 
> be expensive and not desirable. With this, on the client side the object can 
> be constructed back from the map and class name.
> 
> Amareshwari Sriramadasu wrote:
>     > A user can provide a custom implementation of timeline.
>     At run time? We should not be allowing this to be runtime. 
>     
>     > on the client side the object can be constructed back from the map and 
> class name.
>     More implementations can be added on server side and that should not 
> change clients consuming. We can not assume the client accessing to look at 
> this will be schema designer always. The consumers of this data will be much 
> more interested in the timelines than data generators. For ex: we can build a 
> dashboard with the timelines being shown for each table and none of that 
> should worry about implementation of timeline.
> 
> Amareshwari Sriramadasu wrote:
>     And the time line should be given on REST api, and doing anything client 
> side is not much value for users of REST api, mainly for dashboarding.

I feel Ok with single implementation (can't choose which one though, any should 
be ok) on client side being exposed after reading the background. Suggestion: 
in future we will try to put up the design/motivation for user facing features 
on JIRA especially in cases with multiple alternatives. This may avoid the 
confusion later when patch is submitted.


- Puneet


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/45100/#review124722
-----------------------------------------------------------


On March 23, 2016, 2:02 p.m., Ajay Yadava wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45100/
> -----------------------------------------------------------
> 
> (Updated March 23, 2016, 2:02 p.m.)
> 
> 
> Review request for lens.
> 
> 
> Bugs: LENS-819
>     https://issues.apache.org/jira/browse/LENS-819
> 
> 
> Repository: lens
> 
> 
> Description
> -------
> 
> Timeline of the fact returned should have structure instead of String.
> 
> 
> Diffs
> -----
> 
>   lens-api/src/main/java/org/apache/lens/api/TimeLines.java PRE-CREATION 
>   lens-cli/src/main/java/org/apache/lens/cli/commands/LensFactCommands.java 
> 6457bce 
>   lens-cli/src/test/java/org/apache/lens/cli/TestLensFactCommands.java 
> 9670d8f 
>   lens-client/src/main/java/org/apache/lens/client/LensClient.java ea0cd76 
>   lens-client/src/main/java/org/apache/lens/client/LensMetadataClient.java 
> d0f2b57 
>   
> lens-cube/src/main/java/org/apache/lens/cube/metadata/CubeMetastoreClient.java
>  02bbbbd 
>   
> lens-cube/src/main/java/org/apache/lens/cube/metadata/timeline/EndsAndHolesPartitionTimeline.java
>  c588dc7 
>   
> lens-cube/src/main/java/org/apache/lens/cube/metadata/timeline/PartitionTimeline.java
>  1642482 
>   
> lens-cube/src/main/java/org/apache/lens/cube/metadata/timeline/RangesPartitionTimeline.java
>  6c9eb7a 
>   
> lens-cube/src/main/java/org/apache/lens/cube/metadata/timeline/StoreAllPartitionTimeline.java
>  a6cb8da 
>   lens-cube/src/test/java/org/apache/lens/cube/parse/CubeTestSetup.java 
> 42decc6 
>   
> lens-server-api/src/main/java/org/apache/lens/server/api/metastore/CubeMetastoreService.java
>  c11fd83 
>   
> lens-server/src/main/java/org/apache/lens/server/metastore/CubeMetastoreServiceImpl.java
>  a1acd1a 
>   
> lens-server/src/main/java/org/apache/lens/server/metastore/MetastoreResource.java
>  684e2ed 
>   
> lens-server/src/test/java/org/apache/lens/server/metastore/TestMetastoreService.java
>  3b091f5 
> 
> Diff: https://reviews.apache.org/r/45100/diff/
> 
> 
> Testing
> -------
> 
> [INFO] 
> ------------------------------------------------------------------------
> [INFO] Reactor Summary:
> [INFO] 
> [INFO] Lens Checkstyle Rules .............................. SUCCESS [  1.545 
> s]
> [INFO] Lens ............................................... SUCCESS [  2.307 
> s]
> [INFO] Lens API ........................................... SUCCESS [ 20.447 
> s]
> [INFO] Lens API for server and extensions ................. SUCCESS [ 16.574 
> s]
> [INFO] Lens Cube .......................................... SUCCESS [20:52 
> min]
> [INFO] Lens DB storage .................................... SUCCESS [ 38.239 
> s]
> [INFO] Lens Query Library ................................. SUCCESS [ 11.201 
> s]
> [INFO] Lens Hive Driver ................................... SUCCESS [02:40 
> min]
> [INFO] Lens Driver for JDBC ............................... SUCCESS [ 51.661 
> s]
> [INFO] Lens Elastic Search Driver ......................... SUCCESS [ 14.836 
> s]
> [INFO] Lens Server ........................................ SUCCESS [12:42 
> min]
> [INFO] Lens client ........................................ SUCCESS [ 48.640 
> s]
> [INFO] Lens CLI ........................................... SUCCESS [02:27 
> min]
> [INFO] Lens Examples ...................................... SUCCESS [  8.238 
> s]
> [INFO] Lens Ship Jars to Distributed Cache ................ SUCCESS [  0.713 
> s]
> [INFO] Lens Distribution .................................. SUCCESS [  8.405 
> s]
> [INFO] Lens ML Lib ........................................ SUCCESS [01:26 
> min]
> [INFO] Lens ML Ext Distribution ........................... SUCCESS [  1.610 
> s]
> [INFO] Lens Regression .................................... SUCCESS [ 10.090 
> s]
> [INFO] Lens UI ............................................ SUCCESS [ 45.105 
> s]
> [INFO] 
> ------------------------------------------------------------------------
> [INFO] BUILD SUCCESS
> [INFO] 
> ------------------------------------------------------------------------
> [INFO] Total time: 44:49 min
> [INFO] Finished at: 2016-03-21T17:35:07+05:30
> [INFO] Final Memory: 145M/623M
> [INFO] 
> ------------------------------------------------------------------------
> 
> 
> Thanks,
> 
> Ajay Yadava
> 
>

Reply via email to