Skip to content

Make max requests configurable in Azure native FS #22932

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
wendigo merged 1 commit into from
Sep 18, 2024
Merged

Make max requests configurable in Azure native FS #22932

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
17 changes: 17 additions & 0 deletions ...trino-filesystem-azure/src/main/java/io/trino/filesystem/azure/AzureFileSystemConfig.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,8 +14,10 @@
package io.trino.filesystem.azure;

import io.airlift.configuration.Config;
import io.airlift.configuration.ConfigDescription;
import io.airlift.units.DataSize;
import io.airlift.units.DataSize.Unit;
import jakarta.validation.constraints.Min;
import jakarta.validation.constraints.NotEmpty;
import jakarta.validation.constraints.NotNull;

Expand All @@ -34,6 +36,7 @@ public enum AuthType
private DataSize writeBlockSize = DataSize.of(4, Unit.MEGABYTE);
private int maxWriteConcurrency = 8;
private DataSize maxSingleUploadSize = DataSize.of(4, Unit.MEGABYTE);
private Integer maxHttpRequests = 2 * Runtime.getRuntime().availableProcessors();

@NotNull
public AuthType getAuthType()
Expand Down Expand Up @@ -111,4 +114,18 @@ public AzureFileSystemConfig setMaxSingleUploadSize(DataSize maxSingleUploadSize
this.maxSingleUploadSize = maxSingleUploadSize;
return this;
}

@Min(1)
public int getMaxHttpRequests()
{
return maxHttpRequests;
}

@Config("azure.max-http-requests")
@ConfigDescription("Maximum number of concurrent HTTP requests to Azure on every node")
public AzureFileSystemConfig setMaxHttpRequests(int maxHttpRequests)
{
this.maxHttpRequests = maxHttpRequests;
return this;
}
}
17 changes: 10 additions & 7 deletions ...rino-filesystem-azure/src/main/java/io/trino/filesystem/azure/AzureFileSystemFactory.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,8 @@ public AzureFileSystemFactory(OpenTelemetry openTelemetry, AzureAuth azureAuth,
config.getReadBlockSize(),
config.getWriteBlockSize(),
config.getMaxWriteConcurrency(),
config.getMaxSingleUploadSize());
config.getMaxSingleUploadSize(),
config.getMaxHttpRequests());
}

public AzureFileSystemFactory(
Expand All @@ -66,7 +67,8 @@ public AzureFileSystemFactory(
DataSize readBlockSize,
DataSize writeBlockSize,
int maxWriteConcurrency,
DataSize maxSingleUploadSize)
DataSize maxSingleUploadSize,
int maxHttpRequests)
{
this.auth = requireNonNull(azureAuth, "azureAuth is null");
this.endpoint = requireNonNull(endpoint, "endpoint is null");
Expand All @@ -77,7 +79,12 @@ public AzureFileSystemFactory(
this.maxSingleUploadSize = requireNonNull(maxSingleUploadSize, "maxSingleUploadSize is null");
this.tracingOptions = new OpenTelemetryTracingOptions().setOpenTelemetry(openTelemetry);

okHttpClient = new OkHttpClient.Builder().build();
Dispatcher dispatcher = new Dispatcher();
dispatcher.setMaxRequests(maxHttpRequests);
dispatcher.setMaxRequestsPerHost(maxHttpRequests);
Copy link
Copy Markdown
Member

@anusudarsan anusudarsan Aug 26, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

you previously had per-host set to a lower value. does this mean we now need two config options?

Copy link
Copy Markdown
Member Author

@nineinchnick nineinchnick Aug 26, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I didn't have any specific reason for using different values, so I'd rather have one config option.

okHttpClient = new OkHttpClient.Builder()
.dispatcher(dispatcher)
.build();
HttpClientOptions clientOptions = new HttpClientOptions();
clientOptions.setTracingOptions(tracingOptions);
httpClient = createAzureHttpClient(okHttpClient, clientOptions);
Expand All @@ -101,9 +108,6 @@ public static HttpClient createAzureHttpClient(OkHttpClient okHttpClient, HttpCl
Integer poolSize = clientOptions.getMaximumConnectionPoolSize();
// By default, OkHttp uses a maximum idle connection count of 5.
int maximumConnectionPoolSize = (poolSize != null && poolSize > 0) ? poolSize : 5;
Dispatcher dispatcher = new Dispatcher();
dispatcher.setMaxRequests(Runtime.getRuntime().availableProcessors() * 4);
dispatcher.setMaxRequestsPerHost(Runtime.getRuntime().availableProcessors() * 2);

return new OkHttpAsyncHttpClientBuilder(okHttpClient)
.proxy(clientOptions.getProxyOptions())
Expand All @@ -113,7 +117,6 @@ public static HttpClient createAzureHttpClient(OkHttpClient okHttpClient, HttpCl
.readTimeout(clientOptions.getReadTimeout())
.connectionPool(new ConnectionPool(maximumConnectionPoolSize,
clientOptions.getConnectionIdleTimeout().toMillis(), TimeUnit.MILLISECONDS))
.dispatcher(dispatcher)
.build();
}
}
7 changes: 5 additions & 2 deletions ...o-filesystem-azure/src/test/java/io/trino/filesystem/azure/TestAzureFileSystemConfig.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,8 @@ void testDefaults()
.setReadBlockSize(DataSize.of(4, Unit.MEGABYTE))
.setWriteBlockSize(DataSize.of(4, Unit.MEGABYTE))
.setMaxWriteConcurrency(8)
.setMaxSingleUploadSize(DataSize.of(4, Unit.MEGABYTE)));
.setMaxSingleUploadSize(DataSize.of(4, Unit.MEGABYTE))
.setMaxHttpRequests(2 * Runtime.getRuntime().availableProcessors()));
}

@Test
Expand All @@ -49,6 +50,7 @@ public void testExplicitPropertyMappings()
.put("azure.write-block-size", "5MB")
.put("azure.max-write-concurrency", "7")
.put("azure.max-single-upload-size", "7MB")
.put("azure.max-http-requests", "128")
.buildOrThrow();

AzureFileSystemConfig expected = new AzureFileSystemConfig()
Expand All @@ -57,7 +59,8 @@ public void testExplicitPropertyMappings()
.setReadBlockSize(DataSize.of(3, Unit.MEGABYTE))
.setWriteBlockSize(DataSize.of(5, Unit.MEGABYTE))
.setMaxWriteConcurrency(7)
.setMaxSingleUploadSize(DataSize.of(7, Unit.MEGABYTE));
.setMaxSingleUploadSize(DataSize.of(7, Unit.MEGABYTE))
.setMaxHttpRequests(128);

assertFullMapping(properties, expected);
}
Expand Down