bhattmanish98 commented on code in PR #8464:
URL: https://github.com/apache/hadoop/pull/8464#discussion_r3239816463
##########
hadoop-tools/hadoop-azure/src/main/java/org/apache/hadoop/fs/azurebfs/services/AbfsClient.java:
##########
@@ -158,6 +158,7 @@ public abstract class AbfsClient implements Closeable {
private final SharedKeyCredentials sharedKeyCredentials;
private ApiVersion xMsVersion = ApiVersion.getCurrentVersion();
private final ExponentialRetryPolicy exponentialRetryPolicy;
+ private final ExponentialRetryPolicy prefetchRetryPolicy;
Review Comment:
Other places we have used prefetchExponentialRetryPolicy, better to use
prefetchExponentialRetryPolicy as variable name here as well.
##########
hadoop-tools/hadoop-azure/src/main/java/org/apache/hadoop/fs/azurebfs/AbfsConfiguration.java:
##########
@@ -189,6 +189,11 @@ public class AbfsConfiguration{
DefaultValue = DEFAULT_MAX_RETRY_ATTEMPTS)
private int maxIoRetries;
+ @IntegerConfigurationValidatorAnnotation(ConfigurationKey =
AZURE_MAX_IO_PREFETCH_RETRIES,
+ MinValue = 0,
+ DefaultValue = DEFAULT_PREFETCH_MAX_RETRY_ATTEMPTS)
Review Comment:
Generaly we follow pattern for DefaultValue variable as `replace AZURE_ with
DEFAULT_`
##########
hadoop-tools/hadoop-azure/src/main/java/org/apache/hadoop/fs/azurebfs/services/ReadBufferManagerV1.java:
##########
@@ -485,7 +485,9 @@ private int getBlockFromCompletedQueue(final
AbfsInputStream stream, final long
if (buf.getStatus() == ReadBufferStatus.READ_FAILED) {
// To prevent new read requests to fail due to old read-ahead attempts,
// return exception only from buffers that failed within last
thresholdAgeMilliseconds
- if ((currentTimeMillis() - (buf.getTimeStamp()) <
getThresholdAgeMilliseconds())) {
+ if (!isPrefetchRequestPriorityEnabled() && (
Review Comment:
If isPrefetchRequestPriorityEnabled is true, the condition won't be met and
no error will be thrown. Is this the intended behavior?
##########
hadoop-tools/hadoop-azure/src/main/java/org/apache/hadoop/fs/azurebfs/constants/FileSystemConfigurations.java:
##########
@@ -51,6 +51,7 @@ public final class FileSystemConfigurations {
public static final int DEFAULT_STATIC_RETRY_INTERVAL = 1_000; // 1s
public static final int DEFAULT_BACKOFF_INTERVAL = 500; // 500ms
public static final int DEFAULT_MAX_RETRY_ATTEMPTS = 30;
+ public static final int DEFAULT_PREFETCH_MAX_RETRY_ATTEMPTS = 2;
Review Comment:
Any specific reason to choose 2 here? We can add that reason as a comment
for future reference.
##########
hadoop-tools/hadoop-azure/src/main/java/org/apache/hadoop/fs/azurebfs/services/ReadBufferManagerV2.java:
##########
@@ -709,7 +709,7 @@ private int getBlockFromCompletedQueue(final String eTag,
final long position,
if (buf.getStatus() == ReadBufferStatus.READ_FAILED) {
// To prevent new read requests to fail due to old read-ahead attempts,
// return exception only from buffers that failed within last
getThresholdAgeMilliseconds()
- if ((currentTimeMillis() - (buf.getTimeStamp())
+ if (!isPrefetchRequestPriorityEnabled() && (currentTimeMillis() -
(buf.getTimeStamp())
Review Comment:
Same as above
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
