[jira] [Commented] (KAFKA-3940) Log should check the return value of dir.mkdirs()

2017-04-29 Thread ASF GitHub Bot (JIRA) 

[ 
https://issues.apache.org/jira/browse/KAFKA-3940?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15989979#comment-15989979
 ] 

ASF GitHub Bot commented on KAFKA-3940:
---

GitHub user mimaison opened a pull request:

https://github.com/apache/kafka/pull/2944

KAFKA-3940: Replaced File.mkdir/mkdirs/delete by their Files equivalent



You can merge this pull request into a Git repository by running:

$ git pull https://github.com/mimaison/kafka KAFKA-3940

Alternatively you can review and apply these changes as the patch at:

https://github.com/apache/kafka/pull/2944.patch

To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:

This closes #2944


commit 189ead578f8cf99f555aa1de8572d771e7c7b15e
Author: Mickael Maison 
Date:   2017-04-17T17:13:13Z

KAFKA-3940: Replaced File.mkdir/mkdirs/delete by their Files equivalent




> Log should check the return value of dir.mkdirs()
> -
>
> Key: KAFKA-3940
> URL: https://issues.apache.org/jira/browse/KAFKA-3940
> Project: Kafka
>  Issue Type: Bug
>  Components: log
>Affects Versions: 0.10.0.0
>Reporter: Jun Rao
>Assignee: Colin P. McCabe
>  Labels: newbie, reliability
> Fix For: 0.11.0.0
>
>
> In Log.loadSegments(), we call dir.mkdirs() w/o checking the return value and 
> just assume the directory will exist after the call. However, if the 
> directory can't be created (e.g. due to no space), we will hit 
> NullPointerException in the next statement, which will be confusing.
>for(file <- dir.listFiles if file.isFile) {



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

[jira] [Commented] (KAFKA-3940) Log should check the return value of dir.mkdirs()

2017-01-27 Thread ASF GitHub Bot (JIRA) 

[ 
https://issues.apache.org/jira/browse/KAFKA-3940?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15843581#comment-15843581
 ] 

ASF GitHub Bot commented on KAFKA-3940:
---

Github user imandhan closed the pull request at:

https://github.com/apache/kafka/pull/1787


> Log should check the return value of dir.mkdirs()
> -
>
> Key: KAFKA-3940
> URL: https://issues.apache.org/jira/browse/KAFKA-3940
> Project: Kafka
>  Issue Type: Bug
>  Components: log
>Affects Versions: 0.10.0.0
>Reporter: Jun Rao
>Assignee: Ishita Mandhan
>  Labels: newbie, reliability
>
> In Log.loadSegments(), we call dir.mkdirs() w/o checking the return value and 
> just assume the directory will exist after the call. However, if the 
> directory can't be created (e.g. due to no space), we will hit 
> NullPointerException in the next statement, which will be confusing.
>for(file <- dir.listFiles if file.isFile) {



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)

[jira] [Commented] (KAFKA-3940) Log should check the return value of dir.mkdirs()

2016-11-28 Thread Ishita Mandhan (JIRA) 

[ 
https://issues.apache.org/jira/browse/KAFKA-3940?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15701279#comment-15701279
 ] 

Ishita Mandhan commented on KAFKA-3940:
---

[~ijuma] I had a quick question about creating a second PR for this jira as I 
don't know how two PRs that resolve the same jira should be formatted. The 
change in the current PR is to convert dir.mkdirs() to 
Files.createDirectories(dir.toPath). Once this goes in, I want to create a PR 
to convert the file.deletes() to Files.delete(file.toPath). Is there a set 
format for the title that I should follow since it will be the second PR 
resolving the same jira issue? Should I just do something like "KAFKA-3940 Part 
2: Log should check the return value of dir.mkdirs()"? 

> Log should check the return value of dir.mkdirs()
> -
>
> Key: KAFKA-3940
> URL: https://issues.apache.org/jira/browse/KAFKA-3940
> Project: Kafka
>  Issue Type: Bug
>  Components: log
>Affects Versions: 0.10.0.0
>Reporter: Jun Rao
>Assignee: Ishita Mandhan
>  Labels: newbie
>
> In Log.loadSegments(), we call dir.mkdirs() w/o checking the return value and 
> just assume the directory will exist after the call. However, if the 
> directory can't be created (e.g. due to no space), we will hit 
> NullPointerException in the next statement, which will be confusing.
>for(file <- dir.listFiles if file.isFile) {



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)

[jira] [Commented] (KAFKA-3940) Log should check the return value of dir.mkdirs()

2016-08-29 Thread ASF GitHub Bot (JIRA) 

[ 
https://issues.apache.org/jira/browse/KAFKA-3940?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15446621#comment-15446621
 ] 

ASF GitHub Bot commented on KAFKA-3940:
---

GitHub user imandhan reopened a pull request:

https://github.com/apache/kafka/pull/1787

KAFKA-3940 Log should check the return value of dir.mkdirs()



You can merge this pull request into a Git repository by running:

$ git pull https://github.com/imandhan/kafka KAFKA-3940

Alternatively you can review and apply these changes as the patch at:

https://github.com/apache/kafka/pull/1787.patch

To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:

This closes #1787


commit e3f5f293104013da3aa765c9ba7cbdb2553f6485
Author: Ishita Mandhan 
Date:   2016-08-25T21:38:16Z

KAFKA-3940 Log should check the return value of dir.mkdirs()




> Log should check the return value of dir.mkdirs()
> -
>
> Key: KAFKA-3940
> URL: https://issues.apache.org/jira/browse/KAFKA-3940
> Project: Kafka
>  Issue Type: Bug
>  Components: log
>Affects Versions: 0.10.0.0
>Reporter: Jun Rao
>Assignee: Ishita Mandhan
>  Labels: newbie
>
> In Log.loadSegments(), we call dir.mkdirs() w/o checking the return value and 
> just assume the directory will exist after the call. However, if the 
> directory can't be created (e.g. due to no space), we will hit 
> NullPointerException in the next statement, which will be confusing.
>for(file <- dir.listFiles if file.isFile) {



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)

[jira] [Commented] (KAFKA-3940) Log should check the return value of dir.mkdirs()

2016-08-29 Thread ASF GitHub Bot (JIRA) 

[ 
https://issues.apache.org/jira/browse/KAFKA-3940?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15446620#comment-15446620
 ] 

ASF GitHub Bot commented on KAFKA-3940:
---

Github user imandhan closed the pull request at:

https://github.com/apache/kafka/pull/1787


> Log should check the return value of dir.mkdirs()
> -
>
> Key: KAFKA-3940
> URL: https://issues.apache.org/jira/browse/KAFKA-3940
> Project: Kafka
>  Issue Type: Bug
>  Components: log
>Affects Versions: 0.10.0.0
>Reporter: Jun Rao
>Assignee: Ishita Mandhan
>  Labels: newbie
>
> In Log.loadSegments(), we call dir.mkdirs() w/o checking the return value and 
> just assume the directory will exist after the call. However, if the 
> directory can't be created (e.g. due to no space), we will hit 
> NullPointerException in the next statement, which will be confusing.
>for(file <- dir.listFiles if file.isFile) {



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)

[jira] [Commented] (KAFKA-3940) Log should check the return value of dir.mkdirs()

2016-08-25 Thread ASF GitHub Bot (JIRA) 

[ 
https://issues.apache.org/jira/browse/KAFKA-3940?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15437729#comment-15437729
 ] 

ASF GitHub Bot commented on KAFKA-3940:
---

GitHub user imandhan opened a pull request:

https://github.com/apache/kafka/pull/1787

KAFKA-3940 Log should check the return value of dir.mkdirs()



You can merge this pull request into a Git repository by running:

$ git pull https://github.com/imandhan/kafka KAFKA-3940

Alternatively you can review and apply these changes as the patch at:

https://github.com/apache/kafka/pull/1787.patch

To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:

This closes #1787


commit e3f5f293104013da3aa765c9ba7cbdb2553f6485
Author: Ishita Mandhan 
Date:   2016-08-25T21:38:16Z

KAFKA-3940 Log should check the return value of dir.mkdirs()




> Log should check the return value of dir.mkdirs()
> -
>
> Key: KAFKA-3940
> URL: https://issues.apache.org/jira/browse/KAFKA-3940
> Project: Kafka
>  Issue Type: Bug
>  Components: log
>Affects Versions: 0.10.0.0
>Reporter: Jun Rao
>Assignee: Ishita Mandhan
>  Labels: newbie
>
> In Log.loadSegments(), we call dir.mkdirs() w/o checking the return value and 
> just assume the directory will exist after the call. However, if the 
> directory can't be created (e.g. due to no space), we will hit 
> NullPointerException in the next statement, which will be confusing.
>for(file <- dir.listFiles if file.isFile) {



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)

[jira] [Commented] (KAFKA-3940) Log should check the return value of dir.mkdirs()

2016-08-25 Thread ASF GitHub Bot (JIRA) 

[ 
https://issues.apache.org/jira/browse/KAFKA-3940?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15437515#comment-15437515
 ] 

ASF GitHub Bot commented on KAFKA-3940:
---

Github user imandhan closed the pull request at:

https://github.com/apache/kafka/pull/1748


> Log should check the return value of dir.mkdirs()
> -
>
> Key: KAFKA-3940
> URL: https://issues.apache.org/jira/browse/KAFKA-3940
> Project: Kafka
>  Issue Type: Bug
>  Components: log
>Affects Versions: 0.10.0.0
>Reporter: Jun Rao
>Assignee: Ishita Mandhan
>  Labels: newbie
>
> In Log.loadSegments(), we call dir.mkdirs() w/o checking the return value and 
> just assume the directory will exist after the call. However, if the 
> directory can't be created (e.g. due to no space), we will hit 
> NullPointerException in the next statement, which will be confusing.
>for(file <- dir.listFiles if file.isFile) {



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)

[jira] [Commented] (KAFKA-3940) Log should check the return value of dir.mkdirs()

2016-08-23 Thread ASF GitHub Bot (JIRA) 

[ 
https://issues.apache.org/jira/browse/KAFKA-3940?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15434061#comment-15434061
 ] 

ASF GitHub Bot commented on KAFKA-3940:
---

Github user imandhan closed the pull request at:

https://github.com/apache/kafka/pull/1748


> Log should check the return value of dir.mkdirs()
> -
>
> Key: KAFKA-3940
> URL: https://issues.apache.org/jira/browse/KAFKA-3940
> Project: Kafka
>  Issue Type: Bug
>  Components: log
>Affects Versions: 0.10.0.0
>Reporter: Jun Rao
>Assignee: Ishita Mandhan
>  Labels: newbie
>
> In Log.loadSegments(), we call dir.mkdirs() w/o checking the return value and 
> just assume the directory will exist after the call. However, if the 
> directory can't be created (e.g. due to no space), we will hit 
> NullPointerException in the next statement, which will be confusing.
>for(file <- dir.listFiles if file.isFile) {



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)

[jira] [Commented] (KAFKA-3940) Log should check the return value of dir.mkdirs()

2016-08-23 Thread ASF GitHub Bot (JIRA) 

[ 
https://issues.apache.org/jira/browse/KAFKA-3940?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15434062#comment-15434062
 ] 

ASF GitHub Bot commented on KAFKA-3940:
---

GitHub user imandhan reopened a pull request:

https://github.com/apache/kafka/pull/1748

KAFKA-3940 Log should check the return value of dir.mkdirs()

This commit changes all the occurrences of dir.mkdirs() with 
Files.createDirectory(dir.toPath())

You can merge this pull request into a Git repository by running:

$ git pull https://github.com/imandhan/kafka KAFKA-3940

Alternatively you can review and apply these changes as the patch at:

https://github.com/apache/kafka/pull/1748.patch

To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:

This closes #1748


commit baf70497759e636a55d50f03d0c5a9673ac71d2d
Author: Ishita Mandhan 
Date:   2016-08-17T00:32:22Z

KAFKA-3940 Log should check the return value of dir.mkdirs()

This commit changes all the occurrences of dir.mkdirs() with 
Files.createDirectory(dir.toPath())

commit c813091659336a7985353a518b4284ca1bd47cf9
Author: Ishita Mandhan 
Date:   2016-08-17T20:30:46Z

Kafka 3940: Log should check the return value of dir.mkdirs()

Fixed the failing errors in the previous commit that changed occurrences of 
dir.mkdirs() to Files.createDirectory()

commit 825c5e9a70b9781a09e033e6a8951a6319c29ae7
Author: Ishita Mandhan 
Date:   2016-08-24T01:59:16Z

KAFKA-3940 Log should check the return value of dir.mkdirs()




> Log should check the return value of dir.mkdirs()
> -
>
> Key: KAFKA-3940
> URL: https://issues.apache.org/jira/browse/KAFKA-3940
> Project: Kafka
>  Issue Type: Bug
>  Components: log
>Affects Versions: 0.10.0.0
>Reporter: Jun Rao
>Assignee: Ishita Mandhan
>  Labels: newbie
>
> In Log.loadSegments(), we call dir.mkdirs() w/o checking the return value and 
> just assume the directory will exist after the call. However, if the 
> directory can't be created (e.g. due to no space), we will hit 
> NullPointerException in the next statement, which will be confusing.
>for(file <- dir.listFiles if file.isFile) {



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)

[jira] [Commented] (KAFKA-3940) Log should check the return value of dir.mkdirs()

2016-08-16 Thread ASF GitHub Bot (JIRA) 

[ 
https://issues.apache.org/jira/browse/KAFKA-3940?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15423669#comment-15423669
 ] 

ASF GitHub Bot commented on KAFKA-3940:
---

GitHub user imandhan opened a pull request:

https://github.com/apache/kafka/pull/1748

KAFKA-3940 Log should check the return value of dir.mkdirs()

This commit changes all the occurrences of dir.mkdirs() with 
Files.createDirectory(dir.toPath())

You can merge this pull request into a Git repository by running:

$ git pull https://github.com/imandhan/kafka KAFKA-3940

Alternatively you can review and apply these changes as the patch at:

https://github.com/apache/kafka/pull/1748.patch

To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:

This closes #1748


commit baf70497759e636a55d50f03d0c5a9673ac71d2d
Author: Ishita Mandhan 
Date:   2016-08-17T00:32:22Z

KAFKA-3940 Log should check the return value of dir.mkdirs()

This commit changes all the occurrences of dir.mkdirs() with 
Files.createDirectory(dir.toPath())




> Log should check the return value of dir.mkdirs()
> -
>
> Key: KAFKA-3940
> URL: https://issues.apache.org/jira/browse/KAFKA-3940
> Project: Kafka
>  Issue Type: Bug
>  Components: log
>Affects Versions: 0.10.0.0
>Reporter: Jun Rao
>Assignee: Ishita Mandhan
>  Labels: newbie
>
> In Log.loadSegments(), we call dir.mkdirs() w/o checking the return value and 
> just assume the directory will exist after the call. However, if the 
> directory can't be created (e.g. due to no space), we will hit 
> NullPointerException in the next statement, which will be confusing.
>for(file <- dir.listFiles if file.isFile) {



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)

[jira] [Commented] (KAFKA-3940) Log should check the return value of dir.mkdirs()

2016-08-04 Thread Jim Jagielski (JIRA) 

[ 
https://issues.apache.org/jira/browse/KAFKA-3940?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15407632#comment-15407632
 ] 

Jim Jagielski commented on KAFKA-3940:
--

Could you send over the patch... either via an attachment or a GH url?

I agree that Files.createDirectory() is the way to go, since we can leverage 
that fact that we tend to catch all exceptions in a somewhat reasonable manner 
:)

> Log should check the return value of dir.mkdirs()
> -
>
> Key: KAFKA-3940
> URL: https://issues.apache.org/jira/browse/KAFKA-3940
> Project: Kafka
>  Issue Type: Bug
>  Components: log
>Affects Versions: 0.10.0.0
>Reporter: Jun Rao
>Assignee: Ishita Mandhan
>  Labels: newbie
>
> In Log.loadSegments(), we call dir.mkdirs() w/o checking the return value and 
> just assume the directory will exist after the call. However, if the 
> directory can't be created (e.g. due to no space), we will hit 
> NullPointerException in the next statement, which will be confusing.
>for(file <- dir.listFiles if file.isFile) {



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)

[jira] [Commented] (KAFKA-3940) Log should check the return value of dir.mkdirs()

2016-07-19 Thread Ishita Mandhan (JIRA) 

[ 
https://issues.apache.org/jira/browse/KAFKA-3940?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15384663#comment-15384663
 ] 

Ishita Mandhan commented on KAFKA-3940:
---

I think we should convert the dir.mkdirs() to Files.createDirectory. I have a 
patch ready that I can submit and then you can take a look at it and share 
feedback? I'm not sure what's a better way to collaborate but would be open to 
trying out a different method.

> Log should check the return value of dir.mkdirs()
> -
>
> Key: KAFKA-3940
> URL: https://issues.apache.org/jira/browse/KAFKA-3940
> Project: Kafka
>  Issue Type: Bug
>  Components: log
>Affects Versions: 0.10.0.0
>Reporter: Jun Rao
>Assignee: Ishita Mandhan
>  Labels: newbie
>
> In Log.loadSegments(), we call dir.mkdirs() w/o checking the return value and 
> just assume the directory will exist after the call. However, if the 
> directory can't be created (e.g. due to no space), we will hit 
> NullPointerException in the next statement, which will be confusing.
>for(file <- dir.listFiles if file.isFile) {



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)

[jira] [Commented] (KAFKA-3940) Log should check the return value of dir.mkdirs()

2016-07-18 Thread Jim Jagielski (JIRA) 

[ 
https://issues.apache.org/jira/browse/KAFKA-3940?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15382927#comment-15382927
 ] 

Jim Jagielski commented on KAFKA-3940:
--

Let me know if I can add myself to the assignee list. Thx

> Log should check the return value of dir.mkdirs()
> -
>
> Key: KAFKA-3940
> URL: https://issues.apache.org/jira/browse/KAFKA-3940
> Project: Kafka
>  Issue Type: Bug
>  Components: log
>Affects Versions: 0.10.0.0
>Reporter: Jun Rao
>Assignee: Ishita Mandhan
>  Labels: newbie
>
> In Log.loadSegments(), we call dir.mkdirs() w/o checking the return value and 
> just assume the directory will exist after the call. However, if the 
> directory can't be created (e.g. due to no space), we will hit 
> NullPointerException in the next statement, which will be confusing.
>for(file <- dir.listFiles if file.isFile) {



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)

[jira] [Commented] (KAFKA-3940) Log should check the return value of dir.mkdirs()

2016-07-14 Thread Jim Jagielski (JIRA) 

[ 
https://issues.apache.org/jira/browse/KAFKA-3940?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15378180#comment-15378180
 ] 

Jim Jagielski commented on KAFKA-3940:
--

In some places we do a dir.mkdirs() and check the return, and throw an error if 
it fails.

Assuming we want to standardize, should we continue w/ that format or should we 
convert all to using Files.createDirectory() (and a general IOException)? I 
don't think we should mix and match :) 

> Log should check the return value of dir.mkdirs()
> -
>
> Key: KAFKA-3940
> URL: https://issues.apache.org/jira/browse/KAFKA-3940
> Project: Kafka
>  Issue Type: Bug
>  Components: log
>Affects Versions: 0.10.0.0
>Reporter: Jun Rao
>Assignee: Ishita Mandhan
>  Labels: newbie
>
> In Log.loadSegments(), we call dir.mkdirs() w/o checking the return value and 
> just assume the directory will exist after the call. However, if the 
> directory can't be created (e.g. due to no space), we will hit 
> NullPointerException in the next statement, which will be confusing.
>for(file <- dir.listFiles if file.isFile) {



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)

[jira] [Commented] (KAFKA-3940) Log should check the return value of dir.mkdirs()

2016-07-14 Thread ASF GitHub Bot (JIRA) 

[ 
https://issues.apache.org/jira/browse/KAFKA-3940?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15378157#comment-15378157
 ] 

ASF GitHub Bot commented on KAFKA-3940:
---

Github user jimjag closed the pull request at:

https://github.com/apache/kafka/pull/1620


> Log should check the return value of dir.mkdirs()
> -
>
> Key: KAFKA-3940
> URL: https://issues.apache.org/jira/browse/KAFKA-3940
> Project: Kafka
>  Issue Type: Bug
>  Components: log
>Affects Versions: 0.10.0.0
>Reporter: Jun Rao
>Assignee: Ishita Mandhan
>  Labels: newbie
>
> In Log.loadSegments(), we call dir.mkdirs() w/o checking the return value and 
> just assume the directory will exist after the call. However, if the 
> directory can't be created (e.g. due to no space), we will hit 
> NullPointerException in the next statement, which will be confusing.
>for(file <- dir.listFiles if file.isFile) {



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)

[jira] [Commented] (KAFKA-3940) Log should check the return value of dir.mkdirs()

2016-07-14 Thread Jim Jagielski (JIRA) 

[ 
https://issues.apache.org/jira/browse/KAFKA-3940?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15378153#comment-15378153
 ] 

Jim Jagielski commented on KAFKA-3940:
--

I'm fine w/ working w/ Ishita... What I was planning is handling the specific 
problem set and then doing an generalized refactor.

> Log should check the return value of dir.mkdirs()
> -
>
> Key: KAFKA-3940
> URL: https://issues.apache.org/jira/browse/KAFKA-3940
> Project: Kafka
>  Issue Type: Bug
>  Components: log
>Affects Versions: 0.10.0.0
>Reporter: Jun Rao
>Assignee: Ishita Mandhan
>  Labels: newbie
>
> In Log.loadSegments(), we call dir.mkdirs() w/o checking the return value and 
> just assume the directory will exist after the call. However, if the 
> directory can't be created (e.g. due to no space), we will hit 
> NullPointerException in the next statement, which will be confusing.
>for(file <- dir.listFiles if file.isFile) {



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)

[jira] [Commented] (KAFKA-3940) Log should check the return value of dir.mkdirs()

2016-07-13 Thread Ismael Juma (JIRA) 

[ 
https://issues.apache.org/jira/browse/KAFKA-3940?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15375774#comment-15375774
 ] 

Ismael Juma commented on KAFKA-3940:


Yes, I'd prefer if we handled this across the codebase instead of the single 
example. Also, if someone assigns a ticket to themselves, we generally 
recommend that other people don't submit PRs before discussing in the JIRA (to 
avoid duplicated work).

> Log should check the return value of dir.mkdirs()
> -
>
> Key: KAFKA-3940
> URL: https://issues.apache.org/jira/browse/KAFKA-3940
> Project: Kafka
>  Issue Type: Bug
>  Components: log
>Affects Versions: 0.10.0.0
>Reporter: Jun Rao
>Assignee: Ishita Mandhan
>  Labels: newbie
>
> In Log.loadSegments(), we call dir.mkdirs() w/o checking the return value and 
> just assume the directory will exist after the call. However, if the 
> directory can't be created (e.g. due to no space), we will hit 
> NullPointerException in the next statement, which will be confusing.
>for(file <- dir.listFiles if file.isFile) {



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)

[jira] [Commented] (KAFKA-3940) Log should check the return value of dir.mkdirs()

2016-07-13 Thread Ishita Mandhan (JIRA) 

[ 
https://issues.apache.org/jira/browse/KAFKA-3940?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15375692#comment-15375692
 ] 

Ishita Mandhan commented on KAFKA-3940:
---

Hi Jim, I got a notification of your PR because I had this bug assigned to me 
and I was working on it myself. I reviewed your PR and I think the fix would be 
more involved and more occurrences of dir.mkdirs() and File.delete could be 
addressed as discussed above. What are your thoughts? Do you mind collaborating 
on this patch set?

> Log should check the return value of dir.mkdirs()
> -
>
> Key: KAFKA-3940
> URL: https://issues.apache.org/jira/browse/KAFKA-3940
> Project: Kafka
>  Issue Type: Bug
>  Components: log
>Affects Versions: 0.10.0.0
>Reporter: Jun Rao
>Assignee: Ishita Mandhan
>  Labels: newbie
>
> In Log.loadSegments(), we call dir.mkdirs() w/o checking the return value and 
> just assume the directory will exist after the call. However, if the 
> directory can't be created (e.g. due to no space), we will hit 
> NullPointerException in the next statement, which will be confusing.
>for(file <- dir.listFiles if file.isFile) {



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)

[jira] [Commented] (KAFKA-3940) Log should check the return value of dir.mkdirs()

2016-07-13 Thread Jim Jagielski (JIRA) 

[ 
https://issues.apache.org/jira/browse/KAFKA-3940?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15375225#comment-15375225
 ] 

Jim Jagielski commented on KAFKA-3940:
--

Pull request created

> Log should check the return value of dir.mkdirs()
> -
>
> Key: KAFKA-3940
> URL: https://issues.apache.org/jira/browse/KAFKA-3940
> Project: Kafka
>  Issue Type: Bug
>  Components: log
>Affects Versions: 0.10.0.0
>Reporter: Jun Rao
>Assignee: Ishita Mandhan
>  Labels: newbie
>
> In Log.loadSegments(), we call dir.mkdirs() w/o checking the return value and 
> just assume the directory will exist after the call. However, if the 
> directory can't be created (e.g. due to no space), we will hit 
> NullPointerException in the next statement, which will be confusing.
>for(file <- dir.listFiles if file.isFile) {



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)

[jira] [Commented] (KAFKA-3940) Log should check the return value of dir.mkdirs()

2016-07-13 Thread ASF GitHub Bot (JIRA) 

[ 
https://issues.apache.org/jira/browse/KAFKA-3940?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15375222#comment-15375222
 ] 

ASF GitHub Bot commented on KAFKA-3940:
---

GitHub user jimjag opened a pull request:

https://github.com/apache/kafka/pull/1620

KAFKA-3940: Log should check the return value of dir.mkdirs()



You can merge this pull request into a Git repository by running:

$ git pull https://github.com/jimjag/kafka KAFKA-3940

Alternatively you can review and apply these changes as the patch at:

https://github.com/apache/kafka/pull/1620.patch

To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:

This closes #1620


commit b9416916c8dfbcfd623b625b5835fc140c904ac5
Author: Jim Jagielski 
Date:   2016-07-13T15:36:46Z

KAFKA-3940: Log should check the return value of dir.mkdirs()




> Log should check the return value of dir.mkdirs()
> -
>
> Key: KAFKA-3940
> URL: https://issues.apache.org/jira/browse/KAFKA-3940
> Project: Kafka
>  Issue Type: Bug
>  Components: log
>Affects Versions: 0.10.0.0
>Reporter: Jun Rao
>Assignee: Ishita Mandhan
>  Labels: newbie
>
> In Log.loadSegments(), we call dir.mkdirs() w/o checking the return value and 
> just assume the directory will exist after the call. However, if the 
> directory can't be created (e.g. due to no space), we will hit 
> NullPointerException in the next statement, which will be confusing.
>for(file <- dir.listFiles if file.isFile) {



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)

[jira] [Commented] (KAFKA-3940) Log should check the return value of dir.mkdirs()

2016-07-11 Thread Jun Rao (JIRA) 

[ 
https://issues.apache.org/jira/browse/KAFKA-3940?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15372146#comment-15372146
 ] 

Jun Rao commented on KAFKA-3940:


[~imandhan], we don't need to change dir to Files. We just need to change 
occurrences of 
   dir.mkdirs()
to 
   Files.createDirectory()

> Log should check the return value of dir.mkdirs()
> -
>
> Key: KAFKA-3940
> URL: https://issues.apache.org/jira/browse/KAFKA-3940
> Project: Kafka
>  Issue Type: Bug
>  Components: log
>Affects Versions: 0.10.0.0
>Reporter: Jun Rao
>Assignee: Ishita Mandhan
>  Labels: newbie
>
> In Log.loadSegments(), we call dir.mkdirs() w/o checking the return value and 
> just assume the directory will exist after the call. However, if the 
> directory can't be created (e.g. due to no space), we will hit 
> NullPointerException in the next statement, which will be confusing.
>for(file <- dir.listFiles if file.isFile) {



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)

[jira] [Commented] (KAFKA-3940) Log should check the return value of dir.mkdirs()

2016-07-11 Thread Ishita Mandhan (JIRA) 

[ 
https://issues.apache.org/jira/browse/KAFKA-3940?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15372078#comment-15372078
 ] 

Ishita Mandhan commented on KAFKA-3940:
---

If dir is changed from File to Files here 
https://github.com/apache/kafka/blob/trunk/core/src/main/scala/kafka/log/Log.scala#L78,
 several functions will not be available for the type Files and alternative 
implementations will need to be created for each of those calls. Is this what 
you meant or am I misunderstanding something?

> Log should check the return value of dir.mkdirs()
> -
>
> Key: KAFKA-3940
> URL: https://issues.apache.org/jira/browse/KAFKA-3940
> Project: Kafka
>  Issue Type: Bug
>  Components: log
>Affects Versions: 0.10.0.0
>Reporter: Jun Rao
>Assignee: Ishita Mandhan
>  Labels: newbie
>
> In Log.loadSegments(), we call dir.mkdirs() w/o checking the return value and 
> just assume the directory will exist after the call. However, if the 
> directory can't be created (e.g. due to no space), we will hit 
> NullPointerException in the next statement, which will be confusing.
>for(file <- dir.listFiles if file.isFile) {



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)

[jira] [Commented] (KAFKA-3940) Log should check the return value of dir.mkdirs()

2016-07-08 Thread Ismael Juma (JIRA) 

[ 
https://issues.apache.org/jira/browse/KAFKA-3940?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15368797#comment-15368797
 ] 

Ismael Juma commented on KAFKA-3940:


I think we should just change all usages of File.mkdirs() to 
Files.createDirectory(). And similarly for `File.delete` versus `Files.delete`.

> Log should check the return value of dir.mkdirs()
> -
>
> Key: KAFKA-3940
> URL: https://issues.apache.org/jira/browse/KAFKA-3940
> Project: Kafka
>  Issue Type: Bug
>  Components: log
>Affects Versions: 0.10.0.0
>Reporter: Jun Rao
>Assignee: Ishita Mandhan
>  Labels: newbie
>
> In Log.loadSegments(), we call dir.mkdirs() w/o checking the return value and 
> just assume the directory will exist after the call. However, if the 
> directory can't be created (e.g. due to no space), we will hit 
> NullPointerException in the next statement, which will be confusing.
>for(file <- dir.listFiles if file.isFile) {



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)

[jira] [Commented] (KAFKA-3940) Log should check the return value of dir.mkdirs()

2016-07-08 Thread Jun Rao (JIRA) 

[ 
https://issues.apache.org/jira/browse/KAFKA-3940?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15368315#comment-15368315
 ] 

Jun Rao commented on KAFKA-3940:


Also, instead of using File.mkdirs(), it may be better to use 
Files.createDirectory() instead. The latter throws IOException on error instead 
of returning false and will potentially give better indication on the cause.

> Log should check the return value of dir.mkdirs()
> -
>
> Key: KAFKA-3940
> URL: https://issues.apache.org/jira/browse/KAFKA-3940
> Project: Kafka
>  Issue Type: Bug
>  Components: log
>Affects Versions: 0.10.0.0
>Reporter: Jun Rao
>  Labels: newbie
>
> In Log.loadSegments(), we call dir.mkdirs() w/o checking the return value and 
> just assume the directory will exist after the call. However, if the 
> directory can't be created (e.g. due to no space), we will hit 
> NullPointerException in the next statement, which will be confusing.
>for(file <- dir.listFiles if file.isFile) {



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)