Skip to content

Commit

Permalink
feat: update cloudstorageconfiguration to improve copy accross cross-…
Browse files Browse the repository at this point in the history 
…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
  • Loading branch information
@suraj-qlogic
suraj-qlogic committed Jul 27, 2020
1 parent 0e21b76 commit db74524
Show file tree
Hide file tree
Showing 3 changed files with 98 additions and 5 deletions.
32 changes: 27 additions & 5 deletions ...-cloud-nio/src/main/java/com/google/cloud/storage/contrib/nio/CloudStorageFileSystem.java
View file
Expand Up @@ -38,6 +38,8 @@
import java.nio.file.attribute.FileTime;
import java.nio.file.attribute.UserPrincipalLookupService;
import java.util.HashMap;
import java.util.HashSet;
import java.util.Map;
import java.util.Objects;
import java.util.Set;
import javax.annotation.CheckReturnValue;
Expand All @@ -61,10 +63,11 @@ public final class CloudStorageFileSystem extends FileSystem {
public static final int BLOCK_SIZE_DEFAULT = 2 * 1024 * 1024;
public static final FileTime FILE_TIME_UNKNOWN = FileTime.fromMillis(0);
public static final Set<String> SUPPORTED_VIEWS = ImmutableSet.of(BASIC_VIEW, GCS_VIEW);

private final CloudStorageFileSystemProvider provider;
private final String bucket;
private final CloudStorageConfiguration config;
private static final Map<CloudStorageConfiguration, Set<CloudStorageFileSystemProvider>>
CONFIG_TO_PROVIDERS_MAP = new HashMap<>();

// Users can change this: then this affects every filesystem object created
// later, including via SPI. This is meant to be done only once, at the beginning
Expand Down Expand Up @@ -145,7 +148,28 @@ public static CloudStorageFileSystem forBucket(String bucket, CloudStorageConfig
!bucket.startsWith(URI_SCHEME + ":"), "Bucket name must not have schema: %s", bucket);
checkNotNull(config);
return new CloudStorageFileSystem(
new CloudStorageFileSystemProvider(config.userProject()), bucket, config);
getCloudStorageFileSystemProvider(config, null), bucket, config);
}

private static CloudStorageFileSystemProvider getCloudStorageFileSystemProvider(
CloudStorageConfiguration config, StorageOptions storageOptions) {
CloudStorageFileSystemProvider newProvider =
(storageOptions == null)
? new CloudStorageFileSystemProvider(config.userProject())
: new CloudStorageFileSystemProvider(config.userProject(), storageOptions);
Set<CloudStorageFileSystemProvider> existingProviders = CONFIG_TO_PROVIDERS_MAP.get(config);
if (existingProviders == null) {
existingProviders = new HashSet<>();
} else {
for (CloudStorageFileSystemProvider existiningProvider : existingProviders) {
if (existiningProvider.equals(newProvider)) {
return existiningProvider;
}
}
}
existingProviders.add(newProvider);
CONFIG_TO_PROVIDERS_MAP.put(config, existingProviders);
return newProvider;
}

/**
Expand All @@ -169,9 +193,7 @@ public static CloudStorageFileSystem forBucket(
checkArgument(
!bucket.startsWith(URI_SCHEME + ":"), "Bucket name must not have schema: %s", bucket);
return new CloudStorageFileSystem(
new CloudStorageFileSystemProvider(config.userProject(), storageOptions),
bucket,
checkNotNull(config));
getCloudStorageFileSystemProvider(config, storageOptions), bucket, checkNotNull(config));
}

CloudStorageFileSystem(
Expand Down
37 changes: 37 additions & 0 deletions ...ud-nio/src/test/java/com/google/cloud/storage/contrib/nio/CloudStorageFileSystemTest.java
View file
Expand Up @@ -18,6 +18,10 @@

import static com.google.common.truth.Truth.assertThat;
import static java.nio.charset.StandardCharsets.UTF_8;
import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertNotEquals;
import static org.junit.Assert.assertNotSame;
import static org.junit.Assert.assertSame;

import com.google.cloud.storage.StorageOptions;
import com.google.cloud.storage.contrib.nio.testing.LocalStorageHelper;
Expand Down Expand Up @@ -334,6 +338,39 @@ public void testDeleteRecursive() throws IOException {
}
}

@Test
public void testSameProvider() throws IOException {
try (CloudStorageFileSystem sourceFileSystem =
CloudStorageFileSystem.forBucket(
"bucket",
CloudStorageConfiguration.builder().permitEmptyPathComponents(true).build())) {
CloudStorageFileSystem destFileSystem =
CloudStorageFileSystem.forBucket(
"new-bucket",
CloudStorageConfiguration.builder().permitEmptyPathComponents(true).build());
assertSame(sourceFileSystem.provider(), destFileSystem.provider());
assertEquals(sourceFileSystem.config(), destFileSystem.config());
assertEquals("bucket", sourceFileSystem.bucket());
assertEquals("new-bucket", destFileSystem.bucket());
}
}

@Test
public void testDifferentProvider() throws IOException {
try (CloudStorageFileSystem sourceFileSystem =
CloudStorageFileSystem.forBucket(
"bucket",
CloudStorageConfiguration.builder().permitEmptyPathComponents(true).build())) {
CloudStorageFileSystem destFileSystem =
CloudStorageFileSystem.forBucket(
"new-bucket",
CloudStorageConfiguration.builder().permitEmptyPathComponents(false).build());
assertNotSame(sourceFileSystem.provider(), destFileSystem.provider());
assertNotEquals(sourceFileSystem.config(), destFileSystem.config());
assertEquals("bucket", sourceFileSystem.bucket());
assertEquals("new-bucket", destFileSystem.bucket());
}
}
/**
* Delete the given directory and all of its contents if non-empty.
*
Expand Down
34 changes: 34 additions & 0 deletions google-cloud-nio/src/test/java/com/google/cloud/storage/contrib/nio/it/ITGcsNio.java
View file
Expand Up @@ -20,6 +20,10 @@
import static com.google.common.truth.Truth.assertThat;
import static com.google.common.truth.Truth.assertWithMessage;
import static java.nio.charset.StandardCharsets.UTF_8;
import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertNotEquals;
import static org.junit.Assert.assertNotSame;
import static org.junit.Assert.assertSame;

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

private static final Logger log = Logger.getLogger(ITGcsNio.class.getName());
private static final String BUCKET = RemoteStorageHelper.generateBucketName();
private static final String TARGET_BUCKET = RemoteStorageHelper.generateBucketName();
private static final String REQUESTER_PAYS_BUCKET =
RemoteStorageHelper.generateBucketName() + "_rp";
private static final String SML_FILE = "tmp-test-small-file.txt";
Expand All @@ -119,6 +124,7 @@ public static void beforeClass() throws IOException {
storage = storageOptions.getService();
// create and populate test bucket
storage.create(BucketInfo.of(BUCKET));
storage.create(BucketInfo.of(TARGET_BUCKET));
fillFile(storage, BUCKET, SML_FILE, SML_SIZE);
fillFile(storage, BUCKET, BIG_FILE, BIG_SIZE);
BucketInfo requesterPaysBucket =
Expand Down Expand Up @@ -1040,6 +1046,34 @@ public ImmutableList<Path> getPaths() {
}
}

@Test
public void testCopyWithSameProvider() throws IOException {
CloudStorageFileSystem sourceFileSystem = getTestBucket();
CloudStorageFileSystem targetFileSystem =
CloudStorageFileSystem.forBucket(
TARGET_BUCKET, CloudStorageConfiguration.DEFAULT, storageOptions);
Path sourceFileSystemPath = sourceFileSystem.getPath(SML_FILE);
Path targetFileSystemPath = targetFileSystem.getPath(PREFIX + randomSuffix());
Files.copy(sourceFileSystemPath, targetFileSystemPath);
assertSame(sourceFileSystem.provider(), targetFileSystem.provider());
assertEquals(sourceFileSystem.config(), targetFileSystem.config());
}

@Test
public void testCopyWithDifferentProvider() throws IOException {
CloudStorageFileSystem sourceFileSystem = getTestBucket();
CloudStorageFileSystem targetFileSystem =
CloudStorageFileSystem.forBucket(
TARGET_BUCKET,
CloudStorageConfiguration.builder().permitEmptyPathComponents(true).build(),
storageOptions);
Path sourceFileSystemPath = sourceFileSystem.getPath(SML_FILE);
Path targetFileSystemPath = targetFileSystem.getPath(PREFIX + randomSuffix());
Files.copy(sourceFileSystemPath, targetFileSystemPath);
assertNotSame(sourceFileSystem.provider(), targetFileSystem.provider());
assertNotEquals(sourceFileSystem.config(), targetFileSystem.config());
}

private CloudStorageFileSystem getTestBucket() throws IOException {
// in typical usage we use the single-argument version of forBucket
// and rely on the user being logged into their project with the
Expand Down

0 comments on commit db74524

Please sign in to comment.