[HUDI-5241] Optimize HoodieDefaultTimeline API#7241
Conversation
YannByron
force-pushed
the
hudi_dtl
branch
3 times, most recently
from
8b5df16 to
72fc3f6
Compare
|
@hudi-bot run azure |
| Stream<HoodieInstant> getInstants(); | ||
| default Stream<HoodieInstant> getInstantsAsStream() { | ||
| return getInstants().stream(); | ||
| } |
There was a problem hiding this comment.
can we follow the existing convention to keep default impl. in HoodieDefaultTimeline? getInstants() is not implemented at this level hence getInstantsAsStream() may not be optimized to directly call stream(); for e.g. there could be a timeline subclass which is backed by stream of instants so you can return the stream directly instead of getInstants().stream(). let's leave the impl. to subclasses
| default List<HoodieInstant> getAndCopyInstants() { | ||
| return new ArrayList<>(getInstants()); | ||
| } |
There was a problem hiding this comment.
ditto
also we should use new ArrayList<> for getInstants() and never expose instants to users. so we should just have getInstants, and make javadoc clear about the behavior and usage
| Stream<HoodieInstant> getInstantsAsStream(); | ||
|
|
||
| /** | ||
| * @return Get tht list of instants | ||
| */ | ||
| List<HoodieInstant> getInstants(); |
There was a problem hiding this comment.
Should be ok to change. I have checked the usages of HoodieTimeline in other query engines Trino/Presto. We are good.
|
@YannByron Just curious as to how much perf improvements does this change bring? |
It is not about perf improvements. It make api suitable for users' perception, and convenient to extend, like RFC-36(https://cwiki.apache.org/confluence/display/HUDI/RFC-36%3A+HUDI+Metastore+Server) |
6 participants
Change Logs
getInstantstogetInstantsAsStream.getInstantsthat return a list.getInstantsinterface when usingthis.instants.Impact
LOW
Risk level (write none, low medium or high below)
LOW
Documentation Update
Describe any necessary documentation update if there is any new feature, config, or user-facing change
ticket number here and follow the instruction to make
changes to the website.
Contributor's checklist