You are viewing a plain text version of this content. The canonical link for it is here.
Posted to common-issues@hadoop.apache.org by lqjack <gi...@git.apache.org> on 2018/10/23 14:39:26 UTC
[GitHub] hadoop pull request #433: HADOOP-15870
GitHub user lqjack opened a pull request:
https://github.com/apache/hadoop/pull/433
HADOOP-15870
Fix the nextPos
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/lqjack/hadoop HADOOP-15870
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/hadoop/pull/433.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 #433
----
commit ab1b1efb1aa5213c07db18ad29146908ac7c206d
Author: lqjaclee <lq...@...>
Date: 2018-10-23T14:38:02Z
HADOOP-15870
Fix the nextPos
----
---
---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: common-issues-help@hadoop.apache.org
[GitHub] hadoop issue #433: HADOOP-15870
Posted by steveloughran <gi...@git.apache.org>.
Github user steveloughran commented on the issue:
https://github.com/apache/hadoop/pull/433
1. I think the same problem surfaces in `remainingInCurrentRequest()` too, which is where some inspection is going to be needed to carefully understand what's up.
1. For this method, I think maybe we could cut it completely, and fix up `available()` to return a default value when `wrappedStream==null` (0 or maybe 1 except when at EOF), and when the stream isn't null, delegate to it. That pushes down the problem of estimating the number of non-blocking bytes into the http connector.
Tests.
In `AbstractContractSeekTest` there's some existing tests which could take some more asserts on that available call
* `testSeekZeroByteFile`: `available() == 0`, always
* `testSeekReadClosedFile` : call available() on an empty file, it should raise some IllegalStateException or IOE, but not an NPE
For some of the other tests, I think you insert a couple of checks to say " available > 0" after a seek + read() Call. This also clarifies that "what should be available after a seek but before a read()?" As for cloudstores with lazy seek, available == 0, though if that breaks gzip then maybe they should return 1, so the assert should be available >1 with an option in the contract to say "actually returns 0".
It's a tricky one to test on because really, the sole asserts should be
1. fails if the stream is closed (unless its a 0?)
1. returns >= if the stream is open
1. is always < remaining file length.
Probably assert #3 is the one to check for: available() is valid when it is >=0 and <= (filelength -getPos()), with care taken about that second clause to make sure there's no off-by-one errors in the assert/code
---
---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: common-issues-help@hadoop.apache.org
[GitHub] hadoop issue #433: HADOOP-15870
Posted by steveloughran <gi...@git.apache.org>.
Github user steveloughran commented on the issue:
https://github.com/apache/hadoop/pull/433
no worries
---
---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: common-issues-help@hadoop.apache.org
[GitHub] hadoop issue #433: HADOOP-15870
Posted by steveloughran <gi...@git.apache.org>.
Github user steveloughran commented on the issue:
https://github.com/apache/hadoop/pull/433
As the usual test police, which S3 endpoint did you rerun all the hadoop-aws tests against?
https://hadoop.apache.org/docs/current/hadoop-aws/tools/hadoop-aws/testing.html
This only takes < 10 minutes, even remotely
```
mvn verify -Dparallel-tests -DtestsThreadCount=10 -Dscale
# or with s3guard
mvn verify -Dparallel-tests -DtestsThreadCount=10 -Ds3guard -Ddynamodb -Dscale
```
I'll help with testing the other stores (azure wasb/abfs/adl), before the commit, but I'd appreciate the diligence here as it shows you've done the homework. thx
---
---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: common-issues-help@hadoop.apache.org
[GitHub] hadoop issue #433: HADOOP-15870
Posted by lqjack <gi...@git.apache.org>.
Github user lqjack commented on the issue:
https://github.com/apache/hadoop/pull/433
@steveloughran Due to the upgrade to the latest version of the Mac OS, I need take some time to resolve the environment before I can run the test successful .
---
---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: common-issues-help@hadoop.apache.org
[GitHub] hadoop issue #433: HADOOP-15870
Posted by lqjack <gi...@git.apache.org>.
Github user lqjack commented on the issue:
https://github.com/apache/hadoop/pull/433
@steveloughran Thanks very much , I will update.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: common-issues-help@hadoop.apache.org
[GitHub] hadoop issue #433: HADOOP-15870
Posted by lqjack <gi...@git.apache.org>.
Github user lqjack commented on the issue:
https://github.com/apache/hadoop/pull/433
@steveloughran I have updated the code , please help reveiw and comment, thanks .
---
---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: common-issues-help@hadoop.apache.org