Skip to content

Commit 11cdd94

Browse files
suraj-qlogicsuraj-qlogic
authored
feat: update cloudstorageconfiguration to improve copy accross cross-bucket performance (#168)
* feat: update cloudstorageconfiguration to improve cross-bucket performance * feat: update javadoc * feat: fix review changes * feat: fix review changes * feat: fix review changes
1 parent 059f8a9 commit 11cdd94

File tree

3 files changed

+98
-5
lines changed

3 files changed

+98
-5
lines changed

‎java-storage-nio/google-cloud-nio/src/main/java/com/google/cloud/storage/contrib/nio/CloudStorageFileSystem.java‎

Lines changed: 27 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -38,6 +38,8 @@
3838
import java.nio.file.attribute.FileTime;
3939
import java.nio.file.attribute.UserPrincipalLookupService;
4040
import java.util.HashMap;
41+
import java.util.HashSet;
42+
import java.util.Map;
4143
import java.util.Objects;
4244
import java.util.Set;
4345
import javax.annotation.CheckReturnValue;
@@ -61,10 +63,11 @@ public final class CloudStorageFileSystem extends FileSystem {
6163
public static final int BLOCK_SIZE_DEFAULT = 2 * 1024 * 1024;
6264
public static final FileTime FILE_TIME_UNKNOWN = FileTime.fromMillis(0);
6365
public static final Set<String> SUPPORTED_VIEWS = ImmutableSet.of(BASIC_VIEW, GCS_VIEW);
64-
6566
private final CloudStorageFileSystemProvider provider;
6667
private final String bucket;
6768
private final CloudStorageConfiguration config;
69+
private static final Map<CloudStorageConfiguration, Set<CloudStorageFileSystemProvider>>
70+
CONFIG_TO_PROVIDERS_MAP = new HashMap<>();
6871

6972
// Users can change this: then this affects every filesystem object created
7073
// later, including via SPI. This is meant to be done only once, at the beginning
@@ -145,7 +148,28 @@ public static CloudStorageFileSystem forBucket(String bucket, CloudStorageConfig
145148
!bucket.startsWith(URI_SCHEME + ":"), "Bucket name must not have schema: %s", bucket);
146149
checkNotNull(config);
147150
return new CloudStorageFileSystem(
148-
new CloudStorageFileSystemProvider(config.userProject()), bucket, config);
151+
getCloudStorageFileSystemProvider(config, null), bucket, config);
152+
}
153+
154+
private static CloudStorageFileSystemProvider getCloudStorageFileSystemProvider(
155+
CloudStorageConfiguration config, StorageOptions storageOptions) {
156+
CloudStorageFileSystemProvider newProvider =
157+
(storageOptions == null)
158+
? new CloudStorageFileSystemProvider(config.userProject())
159+
: new CloudStorageFileSystemProvider(config.userProject(), storageOptions);
160+
Set<CloudStorageFileSystemProvider> existingProviders = CONFIG_TO_PROVIDERS_MAP.get(config);
161+
if (existingProviders == null) {
162+
existingProviders = new HashSet<>();
163+
} else {
164+
for (CloudStorageFileSystemProvider existiningProvider : existingProviders) {
165+
if (existiningProvider.equals(newProvider)) {
166+
return existiningProvider;
167+
}
168+
}
169+
}
170+
existingProviders.add(newProvider);
171+
CONFIG_TO_PROVIDERS_MAP.put(config, existingProviders);
172+
return newProvider;
149173
}
150174

151175
/**
@@ -169,9 +193,7 @@ public static CloudStorageFileSystem forBucket(
169193
checkArgument(
170194
!bucket.startsWith(URI_SCHEME + ":"), "Bucket name must not have schema: %s", bucket);
171195
return new CloudStorageFileSystem(
172-
new CloudStorageFileSystemProvider(config.userProject(), storageOptions),
173-
bucket,
174-
checkNotNull(config));
196+
getCloudStorageFileSystemProvider(config, storageOptions), bucket, checkNotNull(config));
175197
}
176198

177199
CloudStorageFileSystem(

‎java-storage-nio/google-cloud-nio/src/test/java/com/google/cloud/storage/contrib/nio/CloudStorageFileSystemTest.java‎

Lines changed: 37 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,10 @@
1818

1919
import static com.google.common.truth.Truth.assertThat;
2020
import static java.nio.charset.StandardCharsets.UTF_8;
21+
import static org.junit.Assert.assertEquals;
22+
import static org.junit.Assert.assertNotEquals;
23+
import static org.junit.Assert.assertNotSame;
24+
import static org.junit.Assert.assertSame;
2125

2226
import com.google.cloud.storage.StorageOptions;
2327
import com.google.cloud.storage.contrib.nio.testing.LocalStorageHelper;
@@ -334,6 +338,39 @@ public void testDeleteRecursive() throws IOException {
334338
}
335339
}
336340

341+
@Test
342+
public void testSameProvider() throws IOException {
343+
try (CloudStorageFileSystem sourceFileSystem =
344+
CloudStorageFileSystem.forBucket(
345+
"bucket",
346+
CloudStorageConfiguration.builder().permitEmptyPathComponents(true).build())) {
347+
CloudStorageFileSystem destFileSystem =
348+
CloudStorageFileSystem.forBucket(
349+
"new-bucket",
350+
CloudStorageConfiguration.builder().permitEmptyPathComponents(true).build());
351+
assertSame(sourceFileSystem.provider(), destFileSystem.provider());
352+
assertEquals(sourceFileSystem.config(), destFileSystem.config());
353+
assertEquals("bucket", sourceFileSystem.bucket());
354+
assertEquals("new-bucket", destFileSystem.bucket());
355+
}
356+
}
357+
358+
@Test
359+
public void testDifferentProvider() throws IOException {
360+
try (CloudStorageFileSystem sourceFileSystem =
361+
CloudStorageFileSystem.forBucket(
362+
"bucket",
363+
CloudStorageConfiguration.builder().permitEmptyPathComponents(true).build())) {
364+
CloudStorageFileSystem destFileSystem =
365+
CloudStorageFileSystem.forBucket(
366+
"new-bucket",
367+
CloudStorageConfiguration.builder().permitEmptyPathComponents(false).build());
368+
assertNotSame(sourceFileSystem.provider(), destFileSystem.provider());
369+
assertNotEquals(sourceFileSystem.config(), destFileSystem.config());
370+
assertEquals("bucket", sourceFileSystem.bucket());
371+
assertEquals("new-bucket", destFileSystem.bucket());
372+
}
373+
}
337374
/**
338375
* Delete the given directory and all of its contents if non-empty.
339376
*

‎java-storage-nio/google-cloud-nio/src/test/java/com/google/cloud/storage/contrib/nio/it/ITGcsNio.java‎

Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,10 @@
2020
import static com.google.common.truth.Truth.assertThat;
2121
import static com.google.common.truth.Truth.assertWithMessage;
2222
import static java.nio.charset.StandardCharsets.UTF_8;
23+
import static org.junit.Assert.assertEquals;
24+
import static org.junit.Assert.assertNotEquals;
25+
import static org.junit.Assert.assertNotSame;
26+
import static org.junit.Assert.assertSame;
2327

2428
import com.google.api.client.http.HttpResponseException;
2529
import com.google.cloud.storage.BlobInfo;
@@ -96,6 +100,7 @@ public class ITGcsNio {
96100

97101
private static final Logger log = Logger.getLogger(ITGcsNio.class.getName());
98102
private static final String BUCKET = RemoteStorageHelper.generateBucketName();
103+
private static final String TARGET_BUCKET = RemoteStorageHelper.generateBucketName();
99104
private static final String REQUESTER_PAYS_BUCKET =
100105
RemoteStorageHelper.generateBucketName() + "_rp";
101106
private static final String SML_FILE = "tmp-test-small-file.txt";
@@ -119,6 +124,7 @@ public static void beforeClass() throws IOException {
119124
storage = storageOptions.getService();
120125
// create and populate test bucket
121126
storage.create(BucketInfo.of(BUCKET));
127+
storage.create(BucketInfo.of(TARGET_BUCKET));
122128
fillFile(storage, BUCKET, SML_FILE, SML_SIZE);
123129
fillFile(storage, BUCKET, BIG_FILE, BIG_SIZE);
124130
BucketInfo requesterPaysBucket =
@@ -1040,6 +1046,34 @@ public ImmutableList<Path> getPaths() {
10401046
}
10411047
}
10421048

1049+
@Test
1050+
public void testCopyWithSameProvider() throws IOException {
1051+
CloudStorageFileSystem sourceFileSystem = getTestBucket();
1052+
CloudStorageFileSystem targetFileSystem =
1053+
CloudStorageFileSystem.forBucket(
1054+
TARGET_BUCKET, CloudStorageConfiguration.DEFAULT, storageOptions);
1055+
Path sourceFileSystemPath = sourceFileSystem.getPath(SML_FILE);
1056+
Path targetFileSystemPath = targetFileSystem.getPath(PREFIX + randomSuffix());
1057+
Files.copy(sourceFileSystemPath, targetFileSystemPath);
1058+
assertSame(sourceFileSystem.provider(), targetFileSystem.provider());
1059+
assertEquals(sourceFileSystem.config(), targetFileSystem.config());
1060+
}
1061+
1062+
@Test
1063+
public void testCopyWithDifferentProvider() throws IOException {
1064+
CloudStorageFileSystem sourceFileSystem = getTestBucket();
1065+
CloudStorageFileSystem targetFileSystem =
1066+
CloudStorageFileSystem.forBucket(
1067+
TARGET_BUCKET,
1068+
CloudStorageConfiguration.builder().permitEmptyPathComponents(true).build(),
1069+
storageOptions);
1070+
Path sourceFileSystemPath = sourceFileSystem.getPath(SML_FILE);
1071+
Path targetFileSystemPath = targetFileSystem.getPath(PREFIX + randomSuffix());
1072+
Files.copy(sourceFileSystemPath, targetFileSystemPath);
1073+
assertNotSame(sourceFileSystem.provider(), targetFileSystem.provider());
1074+
assertNotEquals(sourceFileSystem.config(), targetFileSystem.config());
1075+
}
1076+
10431077
private CloudStorageFileSystem getTestBucket() throws IOException {
10441078
// in typical usage we use the single-argument version of forBucket
10451079
// and rely on the user being logged into their project with the

0 commit comments

Comments
 (0)