[jira] [Created] (FLINK-6269) var could be a val

2017-04-05 Thread CanBin Zheng (JIRA)
CanBin Zheng created FLINK-6269:
---

 Summary: var could be a val
 Key: FLINK-6269
 URL: https://issues.apache.org/jira/browse/FLINK-6269
 Project: Flink
  Issue Type: Wish
  Components: JobManager
Affects Versions: 1.2.0
Reporter: CanBin Zheng
Priority: Trivial


In JobManager.scala
var userCodeLoader = libraryCacheManager.getClassLoader(jobGraph.getJobID)
this var could be a val





--
This message was sent by Atlassian JIRA
(v6.3.15#6346)


Re: [DISCUSS] Code style / checkstyle

2017-04-05 Thread Chesnay Schepler
I agree to just allow both. While I definitely prefer 1) i can see why 
someone might prefer 2).


Wouldn't want to delay this anymore; can't find to add this to 
flink-metrics and flink-python...


On 03.04.2017 18:33, Aljoscha Krettek wrote:

I think enough people did already look at the checkstyle rules proposed in the 
PR.

On most of the rules reaching consensus is easy so I propose to enable all 
rules except those regarding placement of curly braces and control flow 
formatting. The two styles in the Flink code base are:

1)
if () {
} else {
}

try {
} catch () {
}

and

2)

if () {
}
else {
}

try {
}
catch () {
}

I think it’s hard to reach consensus on these so I suggest to keep allowing 
both styles.

Any comments very welcome! :-)

Best,
Aljoscha

On 19. Mar 2017, at 17:09, Aljoscha Krettek  wrote:

I played around with this over the week end and it turns out that the required 
changes in flink-streaming-java are not that big. I created a PR with a proposed 
checkstyle.xml and the required code changes: 
https://github.com/apache/flink/pull/3567 
. There’s a longer description of 
the style in the PR. The commits add continuously more invasive changes so we can 
start with the more lightweight changes if we want to.

If we want to go forward with this I would also encourage other people to use 
this for different modules and see how it turns out.

Best,
Aljoscha


On 18 Mar 2017, at 08:00, Aljoscha Krettek mailto:aljos...@apache.org>> wrote:

I added an issue for adding a custom checkstyle.xml for flink-streaming-java so that 
we can gradually add checks: https://issues.apache.org/jira/browse/FLINK-6107 
. I outlined the procedure in 
the Jira. We can use this as a pilot project and see how it goes and then gradually 
also apply to other modules.

What do you think?


On 6 Mar 2017, at 12:42, Stephan Ewen mailto:se...@apache.org>> wrote:

A singular "all reformat in one instant" will cause immense damage to the
project, in my opinion.

- There are so many pull requests that we are having a hard time keeping
up, and merging will a lot more time intensive.
- I personally have many forked branches with WIP features that will
probably never go in if the branches become unmergeable. I expect that to
be true for many other committers and contributors.
- Some companies have Flink forks and are rebasing patches onto master
regularly. They will be completely screwed by a full reformat.

If we do something, the only thing that really is possible is:

(1) Define a style. Ideally not too far away from Flink's style.
(2) Apply it to new projects/modules
(3) Coordinate carefully to pull it into existing modules, module by
module. Leaving time to adopt pull requests bit by bit, and allowing forks
to go through minor merges, rather than total conflict.



On Wed, Mar 1, 2017 at 5:57 PM, Henry Saputra mailto:henry.sapu...@gmail.com>>
wrote:


It is actually Databricks Scala guide to help contributions to Apache Spark
so not really official Spark Scala guide.
The style guide feels bit more like asking people to write Scala in Java
mode so I am -1 to follow the style for Apache Flink Scala if that what you
are recommending.

If the "unification" means ONE style guide for both Java and Scala I would
vote -1 to it because both languages have different semantics and styles to
make them readable and understandable.

We could start with improving the Scala maven style guide to follow more
Scala official style guide [1] and add IntelliJ Idea or Eclipse plugin
style to follow suit.

Java side has bit more strict style check but we could make it tighter but
embracing more Google Java guide closely with minor exceptions

- Henry

[1] http://docs.scala-lang.org/style/ 

On Mon, Feb 27, 2017 at 11:54 AM, Stavros Kontopoulos <
st.kontopou...@gmail.com > wrote:


+1 to provide and enforcing a unified code style for both java and scala.
Unification should apply when it makes sense like comments though.

Eventually code base should be re-factored. I would vote for the one at a
time module fix apporoach.
Style guide should be part of any PR review.

We could also have a look at the spark style guide:
https://github.com/databricks/scala-style-guide 


The style code and general guidelines help keep code more readable and

keep

things simple
with many contributors and different styles of code writing + language
features.


On Mon, Feb 27, 2017 at 8:01 PM, Stephan Ewen mailto:se...@apache.org>> wrote:


I agree, reformatting 90% of the code base is tough.

There are two main issues:
(1) Incompatible merges. This is hard, especially for the folks that

have

to merge the pull requests ;-)

(2) Author history: This is less of an issue, I think. "git log
" and "git show  -- " will still work and

one

may have to go one commit 

Re: [DISCUSS] FLIP-18: Code Generation for improving sorting performance

2017-04-05 Thread Greg Hogan
Pat,

Thanks for running additional tests and continuing to work on this contribution.

My testing is also showing that the performance gains remain even when multiple 
classes are used for sorting.

I think we should proceed in the order of FLINK-3722, FLINK-4705, and 
FLINK-5734. Gabor has reviewed FLINK-3722 and I’ve done so multiple times. I’m 
looking into test coverage for FLINK-4705. Once these are reviewed and 
FLINK-5734 rebased we can benchmark Flink’s performance to validate the 
improvements.

Greg


> On Apr 3, 2017, at 8:46 PM, Pattarawat Chormai  wrote:
> 
> Hi guys,
> 
> I have made an additional optimization[1] related to megamorphic call issue
> that Greg mentioned earlier. The optimization[2] improves execution time
> around ~13%, while the original code from FLINK-5734 is ~11%.
> 
> IMHO, the improvement from metamorphic call optimization is very small
> compared to the code we have to introduce. So, I think we can just go with
> the PR that we currently have. What do you think?
> 
> [1]
> https://github.com/heytitle/flink/commit/8e38b4d738b9953337361c62a8d77e909327d28f
> [2]https://docs.google.com/spreadsheets/d/1PcdCdFX4bGecO6Lb5dLI2nww2NoeaA8c3MgbEdsVmk0/edit#gid=598217386
> 
> Best,
> Pat
> 
> 
> 
> --
> View this message in context: 
> http://apache-flink-mailing-list-archive.1008284.n3.nabble.com/DISCUSS-FLIP-18-Code-Generation-for-improving-sorting-performance-tp16486p16923.html
> Sent from the Apache Flink Mailing List archive. mailing list archive at 
> Nabble.com.



Re: [DISCUSS] Code style / checkstyle

2017-04-05 Thread Greg Hogan
Could we use Aljoscha’s proposed checkstyle [0] as a baseline for discussion? 
It does not define a line length and we could bid down from there.

Chesnay not only reviewed the changes but summarized the checkstyle [1]:

It enforces the following rules:
• every file has to end with a new-line
• no trailing whitespace anywhere
• every public/protected method/class must have a javadoc
• imports have to be sorted alphabetically
• static imports must come first
• whitespace after commas, semicolons and casts
• whitespace around operators, if, for, etc.
• left curly brace ({) must not be at the start of the line
• else/catch/finally must be on the same line as the right curly brace 
(})
• static final variables must be upper case
• non-static variables must not be upper case

[0] 
https://github.com/aljoscha/flink/blob/b60e1b8885beafe46eb7ec2552aa71cfe175aa95/tools/maven/strict-checkstyle.xml
[1] https://github.com/apache/flink/pull/3567#issuecomment-287764234 



> On Apr 4, 2017, at 11:55 PM, Jinkui Shi  wrote:
> 
> It’s glad to restart check style discuss.
> I’m agree with Stephan’s strategy.
> 
> For the ongoing partial pull request and companies’ fork, need to be rewrite 
> following new style rule. That must be finished themselves.  
> 
> We can discuss the check style rule detail one by one(checkstyle.xml, 
> scalastyle-config.xml). The rules should be accept by most of developers.
> 
> I provide one rule to discuss:
> - maxLineLength: 120(java and scala)
> 
> [1]https://issues.apache.org/jira/browse/FLINK-4519 
> 
>> 在 2017年3月6日,下午7:42,Stephan Ewen  写道:
>> 
>> A singular "all reformat in one instant" will cause immense damage to the
>> project, in my opinion.
>> 
>> - There are so many pull requests that we are having a hard time keeping
>> up, and merging will a lot more time intensive.
>> - I personally have many forked branches with WIP features that will
>> probably never go in if the branches become unmergeable. I expect that to
>> be true for many other committers and contributors.
>> - Some companies have Flink forks and are rebasing patches onto master
>> regularly. They will be completely screwed by a full reformat.
>> 
>> If we do something, the only thing that really is possible is:
>> 
>> (1) Define a style. Ideally not too far away from Flink's style.
>> (2) Apply it to new projects/modules
>> (3) Coordinate carefully to pull it into existing modules, module by
>> module. Leaving time to adopt pull requests bit by bit, and allowing forks
>> to go through minor merges, rather than total conflict.
>> 
>> 
>> 
>> On Wed, Mar 1, 2017 at 5:57 PM, Henry Saputra 
>> wrote:
>> 
>>> It is actually Databricks Scala guide to help contributions to Apache Spark
>>> so not really official Spark Scala guide.
>>> The style guide feels bit more like asking people to write Scala in Java
>>> mode so I am -1 to follow the style for Apache Flink Scala if that what you
>>> are recommending.
>>> 
>>> If the "unification" means ONE style guide for both Java and Scala I would
>>> vote -1 to it because both languages have different semantics and styles to
>>> make them readable and understandable.
>>> 
>>> We could start with improving the Scala maven style guide to follow more
>>> Scala official style guide [1] and add IntelliJ Idea or Eclipse plugin
>>> style to follow suit.
>>> 
>>> Java side has bit more strict style check but we could make it tighter but
>>> embracing more Google Java guide closely with minor exceptions
>>> 
>>> - Henry
>>> 
>>> [1] http://docs.scala-lang.org/style/
>>> 
>>> On Mon, Feb 27, 2017 at 11:54 AM, Stavros Kontopoulos <
>>> st.kontopou...@gmail.com> wrote:
>>> 
 +1 to provide and enforcing a unified code style for both java and scala.
 Unification should apply when it makes sense like comments though.
 
 Eventually code base should be re-factored. I would vote for the one at a
 time module fix apporoach.
 Style guide should be part of any PR review.
 
 We could also have a look at the spark style guide:
 https://github.com/databricks/scala-style-guide
 
 The style code and general guidelines help keep code more readable and
>>> keep
 things simple
 with many contributors and different styles of code writing + language
 features.
 
 
 On Mon, Feb 27, 2017 at 8:01 PM, Stephan Ewen  wrote:
 
> I agree, reformatting 90% of the code base is tough.
> 
> There are two main issues:
> (1) Incompatible merges. This is hard, especially for the folks that
 have
> to merge the pull requests ;-)
> 
> (2) Author history: This is less of an issue, I think. "git log
> " and "git show  -- " will still work and
 one
> may have to go one commit back to find out why something was changed
>>>

[jira] [Created] (FLINK-6268) Object reuse for Either type

2017-04-05 Thread Greg Hogan (JIRA)
Greg Hogan created FLINK-6268:
-

 Summary: Object reuse for Either type
 Key: FLINK-6268
 URL: https://issues.apache.org/jira/browse/FLINK-6268
 Project: Flink
  Issue Type: Improvement
  Components: Core
Affects Versions: 1.3.0
Reporter: Greg Hogan
Assignee: Greg Hogan
Priority: Minor


While reviewing test coverage for FLINK-4705 I have come across that {{Either}} 
only implements partial object reuse (when from and to are both {{Right}}). We 
can implement full object reuse if {{Left}} stores a reference to a {{Right}} 
and {{Right}} to a {{Left}}. These references will be {{private}} and will 
remain {{null}} until set by {{EitherSerializer}} when copying or deserializing 
with object reuse.



--
This message was sent by Atlassian JIRA
(v6.3.15#6346)


Re: [VOTE] Release Apache Flink 1.2.1 (RC1)

2017-04-05 Thread Ufuk Celebi
@Stefan: What's the state with the RocksDB fixes? I would be +1 to do this.

On Tue, Apr 4, 2017 at 6:05 PM, Chesnay Schepler  wrote:
> Yes, aljoscha already opened one against master:
> https://github.com/apache/flink/pull/3670
>
> On 04.04.2017 17:57, Ted Yu wrote:
>>
>> Should the commits be reverted from master branch as well ?
>>
>> On Tue, Apr 4, 2017 at 4:59 AM, Aljoscha Krettek 
>> wrote:
>>
>>> The commits around FLINK-5808 have been reverted on release-1.2.
>>>
 On 4. Apr 2017, at 12:16, Stefan Richter 
>>>
>>> wrote:

 I have created a custom build of RocksDB 4.11.2 that fixes a significant
>>>
>>> performance problem with append operations. I think this should
>>> definitely
>>> be part of the 1.2.1 release because this is already blocking some users.
>>> What is missing is uploading the jar to maven central and a testing run,
>>> e.g. with some misbehaved job that has large state.


> Am 04.04.2017 um 11:57 schrieb Robert Metzger :
>
> Thank you for opening a PR for this.
>
> Chesnay, do you need more reviews for the metrics changes / backports?
>
> Are there any other release blockers for 1.2.1, or are we good to go?
>
> On Mon, Apr 3, 2017 at 6:48 PM, Aljoscha Krettek 
> wrote:
>
>> I created a PR for the revert: https://github.com/apache/
>>>
>>> flink/pull/3664
>>>
>>> On 3. Apr 2017, at 18:32, Stephan Ewen  wrote:
>>>
>>> +1 for options (1), but also invest the time to fix it properly for
>>>
>>> 1.2.2
>>>
>>>
>>> On Mon, Apr 3, 2017 at 9:10 AM, Kostas Kloudas <
>>
>> k.klou...@data-artisans.com>
>>>
>>> wrote:
>>>
 +1 for 1

> On Apr 3, 2017, at 5:52 PM, Till Rohrmann 
>>
>> wrote:
>
> +1 for option 1)
>
> On Mon, Apr 3, 2017 at 5:48 PM, Fabian Hueske 
>>
>> wrote:
>>
>> +1 to option 1)
>>
>> 2017-04-03 16:57 GMT+02:00 Ted Yu :
>>
>>> Looks like #1 is better - 1.2.1 would be at least as stable as
>>>
>>> 1.2.0
>>>
>>> Cheers
>>>
>>> On Mon, Apr 3, 2017 at 7:39 AM, Aljoscha Krettek <
>>
>> aljos...@apache.org>
>>>
>>> wrote:
>>>
 Just so we’re all on the same page. ;-)

 There was https://issues.apache.org/jira/browse/FLINK-5808 which
>>
>> was

 a

 bug that we initially discovered in Flink 1.2 which was/is about
>>
>> missing

 verification for the correctness of the combination of
>>>
>>> parallelism
>>
>> and

 max-parallelism. Due to lacking test coverage this introduced
 two
>>
>> more
>>>
>>> bugs:

 - https://issues.apache.org/jira/browse/FLINK-6188: Some
 setParallelism() methods can't cope with default parallelism
 - https://issues.apache.org/jira/browse/FLINK-6209:
 StreamPlanEnvironment always has a parallelism of 1

 IMHO, the options are:
 1) revert the changes made for FLINK-5808 on the release-1.2
>>>
>>> branch
>>
>> and

 live with the bug still being present
 2) put in more work to fix FLINK-5808 which requires fixing some
>>>
>>> problems

 that have existed for a long time with how the parallelism is
>>>
>>> set in

 streaming programs

 Best,
 Aljoscha

> On 31. Mar 2017, at 21:34, Robert Metzger 
>>
>> wrote:
>
> I don't know what is best to do, but I think releasing 1.2.1
>>>
>>> with
>
> potentially more bugs than 1.2.0 is not a good option.
> I suspect a good workaround for FLINK-6188
>  is setting
>>>
>>> the
>
> parallelism manually for operators that can't cope with the
>>>
>>> default
>>
>> -1
>
> parallelism.
>
> On Fri, Mar 31, 2017 at 9:06 PM, Aljoscha Krettek <
>>
>> aljos...@apache.org
>
> wrote:
>
>> You mean reverting the changes around FLINK-5808 [1]? This is
>>>
>>> what
>>
>> introduced the follow-up FLINK-6188 [2].
>>
>> [1] https://issues.apache.org/jira/browse/FLINK-5808
>> [2]https://issues.apache.org/jira/browse/FLINK-6188
>>
>> On Fri, Mar 31, 2017, at 19:10, Robert Metzger wrote:
>>>
>>> I think reverting FLINK-6188 for the 1.2 branch might be a
>>>
>>> good
>>
>> idea.
>>>
>

[jira] [Created] (FLINK-6267) Remove the useless import in FlinkRelBuilder

2017-04-05 Thread sunjincheng (JIRA)
sunjincheng created FLINK-6267:
--

 Summary: Remove the useless import in FlinkRelBuilder
 Key: FLINK-6267
 URL: https://issues.apache.org/jira/browse/FLINK-6267
 Project: Flink
  Issue Type: Improvement
  Components: Table API & SQL
Reporter: sunjincheng
Assignee: sunjincheng


Remove FLINK-6037 legacy useless import.



--
This message was sent by Atlassian JIRA
(v6.3.15#6346)


[jira] [Created] (FLINK-6266) Remove the useless import in FlinkRelBuilder

2017-04-05 Thread sunjincheng (JIRA)
sunjincheng created FLINK-6266:
--

 Summary: Remove the useless import in FlinkRelBuilder
 Key: FLINK-6266
 URL: https://issues.apache.org/jira/browse/FLINK-6266
 Project: Flink
  Issue Type: Improvement
  Components: Table API & SQL
Reporter: sunjincheng
Assignee: sunjincheng


Remove FLINK-6037 legacy useless import.



--
This message was sent by Atlassian JIRA
(v6.3.15#6346)