BJ-698: fix "Timeout waiting for connection from pool" with close IO (#20)

src/main/java/com/rbkmoney/file/storage/service/AmazonS3StorageService.java

@@ -24,6 +24,7 @@ import org.springframework.stereotype.Service;
 import javax.annotation.PostConstruct;
 import javax.annotation.PreDestroy;
 import java.io.ByteArrayInputStream;
+import java.io.IOException;
 import java.io.InputStream;
 import java.net.URL;
 import java.time.Instant;
@@ -64,11 +65,10 @@ public class AmazonS3StorageService implements StorageService {
         FileDto fileDto = fileDto(fileDataId, fileId, metadata);
 
         // записывается неизменяемый фейковый файл с метаданными, в котором находится ссылка на реальный файл
-        log.info("Upload fake metadata file, fileDataId='{}', bucketId='{}'", fileDataId, bucketName);
+        uploadEmptyFileWithMetadata(fileDataId, fileDto);
-        uploadFakeMetadataFile(fileDataId, fileDto);
 
         // генерируется ссылка на выгрузку файла в хранилище напрямую в цеф по ключу fileId
-        log.info("Generate Upload Url for real file, fileDataId='{}', bucketId='{}'", fileDataId, bucketName);
+        log.info("Generate Upload Url, fileDataId='{}', bucketId='{}'", fileDataId, bucketName);
         URL uploadUrl = generatePresignedUrl(fileDataId, fileId, expirationTime, HttpMethod.PUT);
 
         log.info("NewFileResult has been successfully created, fileDataId='{}', bucketId='{}'", fileDataId, bucketName);
@@ -81,11 +81,11 @@ public class AmazonS3StorageService implements StorageService {
         log.info("Trying to generate Download Url, fileDataId='{}', bucketId='{}'", fileDataId, bucketName);
 
         // достается неизменяемый фейковый файл с метаданными
-        log.info("Extract id of real file from fake metadata file, fileDataId='{}', bucketId='{}'", fileDataId, bucketName);
+        log.info("Extract file, fileDataId='{}', bucketId='{}'", fileDataId, bucketName);
         FileDto fileDto = getFileDto(fileDataId);
 
         // генерируем ссылку на загрузку файла из хранилища напрямую в цеф по ключу fileId
-        log.info("Generate Download Url for real file, fileDataId='{}', bucketId='{}'", fileDataId, bucketName);
+        log.info("Generate Download Url, fileDataId='{}', bucketId='{}'", fileDataId, bucketName);
         URL generatePresignedUrl = generatePresignedUrl(fileDto.getFileDataId(), fileDto.getFileId(), expirationTime, HttpMethod.GET);
 
         log.info("Download Url has been successfully generate, fileDataId='{}', bucketId='{}'", fileDataId, bucketName);
@@ -98,15 +98,11 @@ public class AmazonS3StorageService implements StorageService {
         log.info("Trying to get FileData, fileDataId='{}', bucketId='{}'", fileDataId, bucketName);
 
         // достается неизменяемый фейковый файл с метаданными
-        log.info("Extract id of real file from fake metadata file, fileDataId='{}', bucketId='{}'", fileDataId, bucketName);
+        log.info("Extract file, fileDataId='{}', bucketId='{}'", fileDataId, bucketName);
         FileDto fileDto = getFileDto(fileDataId);
 
         // достается реальный файл формата s3
-        log.info("Extract real file, fileDataId='{}', bucketId='{}'", fileDataId, bucketName);
+        String fileName = getFileName(fileDataId, fileDto);
-        S3Object object = getS3Object(fileDataId, fileDto.getFileId());
-
-        log.info("Extract file name, fileDataId='{}', bucketId='{}'", fileDataId, bucketName);
-        String fileName = extractFileName(object);
 
         log.info("FileData has been successfully got, fileDataId='{}', bucketId='{}'", fileDataId, bucketName);
 
@@ -131,7 +127,7 @@ public class AmazonS3StorageService implements StorageService {
         }
     }
 
-    private void uploadFakeMetadataFile(String fileDataId, FileDto fileDto) {
+    private void uploadEmptyFileWithMetadata(String fileDataId, FileDto fileDto) {
         try {
             PutObjectRequest putObjectRequest = putObjectRequest(fileDataId, fileDto, byteArrayInputStream());
 
@@ -142,13 +138,13 @@ public class AmazonS3StorageService implements StorageService {
             Thread.currentThread().interrupt();
             throw new WaitingUploadException(
                     format(
-                            "Thread is interrupted while waiting for the fake metadata file upload to complete, fileDataId=%s, bucketId=%s",
+                            "Thread is interrupted while waiting for the file upload to complete, fileDataId=%s, bucketId=%s",
                             fileDataId, bucketName
                     )
             );
         } catch (SdkBaseException ex) {
             throw new StorageException(
-                    format("Failed to upload fake metadata file, fileDataId=%s, bucketId=%s", fileDataId, bucketName),
+                    format("Failed to upload file, fileDataId=%s, bucketId=%s", fileDataId, bucketName),
                     ex
             );
         }
@@ -176,12 +172,17 @@ public class AmazonS3StorageService implements StorageService {
 
         checkRealFileStatus(fileDataId, s3Object);
 
-        return getFileDtoByFakeFile(fileDataId, s3Object.getObjectMetadata());
+        return getFileDto(fileDataId, s3Object.getObjectMetadata());
+    }
+
+    private String getFileName(String fileDataId, FileDto fileDto) {
+        S3Object s3Object = getS3Object(fileDataId, fileDto.getFileId());
+
+        return extractFileName(s3Object);
     }
 
     private S3Object getS3Object(String fileDataId, String id) {
-        try {
+        try (S3Object object = s3Client.getObject(new GetObjectRequest(bucketName, id))) {
-            S3Object object = s3Client.getObject(new GetObjectRequest(bucketName, id));
 
             checkNotNull("S3Object", fileDataId, object);
 
@@ -191,6 +192,8 @@ public class AmazonS3StorageService implements StorageService {
                     format("Failed to get S3Object, fileDataId=%s, bucketId=%s", fileDataId, bucketName),
                     ex
             );
+        } catch (IOException ex) {
+            throw new StorageException(format("Unable to close S3Object, fileDataId=%s, bucketId=%s", fileDataId, bucketName), ex);
         }
     }
 
@@ -213,7 +216,7 @@ public class AmazonS3StorageService implements StorageService {
         throw new FileNotFoundException(format("S3Object is null, fileDataId=%s, bucketId=%s", fileDataId, bucketName));
     }
 
-    private FileDto getFileDtoByFakeFile(String fileDataId, ObjectMetadata objectMetadata) {
+    private FileDto getFileDto(String fileDataId, ObjectMetadata objectMetadata) {
         String id = getUserMetadataParameter(fileDataId, objectMetadata, FILE_DATA_ID);
         String fileId = getFileIdFromObjectMetadata(fileDataId, objectMetadata);
         String createdAt = getUserMetadataParameter(fileDataId, objectMetadata, CREATED_AT);
@@ -244,8 +247,8 @@ public class AmazonS3StorageService implements StorageService {
                 );
     }
 
-    private String extractFileName(S3Object object) {
+    private String extractFileName(S3Object s3Object) {
-        String contentDisposition = object.getObjectMetadata().getContentDisposition();
+        String contentDisposition = s3Object.getObjectMetadata().getContentDisposition();
         int fileNameIndex = contentDisposition.lastIndexOf(FILENAME_PARAM) + FILENAME_PARAM.length();
         return contentDisposition.substring(fileNameIndex);
     }

src/test/java/com/rbkmoney/file/storage/FileStorageTest.java

@@ -7,7 +7,6 @@ import org.apache.http.client.methods.HttpPut;
 import org.apache.http.entity.FileEntity;
 import org.apache.http.impl.client.HttpClientBuilder;
 import org.apache.thrift.TException;
-import org.junit.Assert;
 import org.junit.Test;
 import org.springframework.http.HttpStatus;
 
@@ -21,9 +20,12 @@ import java.nio.file.Path;
 import java.nio.file.Paths;
 import java.nio.file.StandardCopyOption;
 import java.util.*;
+import java.util.concurrent.CountDownLatch;
+import java.util.concurrent.ExecutorService;
+import java.util.concurrent.Executors;
 
 import static com.rbkmoney.file.storage.msgpack.Value.*;
-import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.*;
 import static org.junit.jupiter.api.Assertions.assertThrows;
 
 // все тесты в 1 классе , чтобы сэкономить время на поднятии тест контейнера
@@ -46,14 +48,14 @@ public class FileStorageTest extends AbstractIntegrationTest {
         requestPut.setEntity(new FileEntity(path.toFile()));
 
         HttpResponse response = httpClient.execute(requestPut);
-        Assert.assertEquals(response.getStatusLine().getStatusCode(), org.apache.http.HttpStatus.SC_OK);
+        assertEquals(response.getStatusLine().getStatusCode(), org.apache.http.HttpStatus.SC_OK);
 
         // генерация url с доступом только для загрузки
         String downloadUrl = fileStorageClient.generateDownloadUrl(fileResult.getFileDataId(), expirationTime);
 
         HttpResponse responseGet = httpClient.execute(new HttpGet(downloadUrl));
         InputStream content = responseGet.getEntity().getContent();
-        Assert.assertEquals(getContent(Files.newInputStream(path)), getContent(content));
+        assertEquals(getContent(Files.newInputStream(path)), getContent(content));
     }
 
     @Test
@@ -216,6 +218,48 @@ public class FileStorageTest extends AbstractIntegrationTest {
         assertEquals(fileName, URLDecoder.decode(fileData.getFileName(), StandardCharsets.UTF_8.name()));
     }
 
+    @Test
+    public void s3ConnectionPoolTest() throws Exception {
+        String expirationTime = generateCurrentTimePlusDay().toString();
+        HttpClient httpClient = HttpClientBuilder.create().build();
+
+        NewFileResult fileResult = fileStorageClient.createNewFile(Collections.emptyMap(), expirationTime);
+
+        Path path = getFileFromResources();
+
+        HttpPut requestPut = new HttpPut(fileResult.getUploadUrl());
+        requestPut.setHeader("Content-Disposition", "attachment;filename=" + URLEncoder.encode(FILE_NAME, StandardCharsets.UTF_8.name()));
+        requestPut.setEntity(new FileEntity(path.toFile()));
+
+        HttpResponse response = httpClient.execute(requestPut);
+        assertEquals(response.getStatusLine().getStatusCode(), org.apache.http.HttpStatus.SC_OK);
+
+        // генерация url с доступом только для загрузки
+        String downloadUrl = fileStorageClient.generateDownloadUrl(fileResult.getFileDataId(), expirationTime);
+
+        HttpResponse responseGet = httpClient.execute(new HttpGet(downloadUrl));
+        InputStream content = responseGet.getEntity().getContent();
+        assertEquals(getContent(Files.newInputStream(path)), getContent(content));
+
+        CountDownLatch countDownLatch = new CountDownLatch(1000);
+        ExecutorService executor = Executors.newFixedThreadPool(5);
+        for (int i = 0; i < 1000; i++) {
+            executor.execute(
+                    () -> {
+                        try {
+                            fileStorageClient.getFileData(fileResult.getFileDataId());
+                            countDownLatch.countDown();
+                        } catch (TException fileNotFound) {
+                            fail();
+                        }
+                    }
+            );
+        }
+
+        countDownLatch.await();
+        assertTrue(true);
+    }
+
     private void uploadTestData(NewFileResult fileResult, String fileName, String testData) throws IOException {
         // запись данных методом put
         URL uploadUrl = new URL(fileResult.getUploadUrl());

src/test/resources/logback-test.xml

@@ -19,12 +19,16 @@
         <level value="error"/>
     </logger>
 
-    <logger name="com.amazonaws">
-        <level value="error"/>
-    </logger>
-
     <logger name="org.apache.http">
         <level value="error"/>
     </logger>
 
+    <logger name="com.amazonaws">
+        <level value="error"/>
+    </logger>
+
+    <logger name="com.rbkmoney.woody">
+        <level value="error"/>
+    </logger>
+
 </configuration>