From f896e354ee03be2e2e0ee66c76eed141408e5cb2 Mon Sep 17 00:00:00 2001 From: Felix Jancso-Szabo Date: Tue, 2 Apr 2024 11:16:24 -0400 Subject: [PATCH 1/6] Consume all result pages in DynamoDbLogicalDb.batchLoad --- .../kotlin/app/cash/tempest2/internal/DynamoDbLogicalDb.kt | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tempest2/src/main/kotlin/app/cash/tempest2/internal/DynamoDbLogicalDb.kt b/tempest2/src/main/kotlin/app/cash/tempest2/internal/DynamoDbLogicalDb.kt index 7384ef3e..6e275232 100644 --- a/tempest2/src/main/kotlin/app/cash/tempest2/internal/DynamoDbLogicalDb.kt +++ b/tempest2/src/main/kotlin/app/cash/tempest2/internal/DynamoDbLogicalDb.kt @@ -101,8 +101,8 @@ internal class DynamoDbLogicalDb( returnConsumedCapacity ) - val pages = batchRequests.map { - dynamoDbEnhancedClient.batchGetItem(it).iterator().next() + val pages = batchRequests.flatMap { + dynamoDbEnhancedClient.batchGetItem(it).iterator().asSequence().toList() } return toBatchLoadResponse(keysByTable, requestKeys, pages) From 3603831a44a889ef5c9c446482397a30c04b6c84 Mon Sep 17 00:00:00 2001 From: Felix Jancso-Szabo Date: Tue, 2 Apr 2024 11:53:34 -0400 Subject: [PATCH 2/6] Add test --- .../app/cash/tempest2/LogicalDbBatchTest.kt | 27 +++++++++++++++++++ 1 file changed, 27 insertions(+) diff --git a/tempest2/src/test/kotlin/app/cash/tempest2/LogicalDbBatchTest.kt b/tempest2/src/test/kotlin/app/cash/tempest2/LogicalDbBatchTest.kt index 59fa0d36..04583762 100644 --- a/tempest2/src/test/kotlin/app/cash/tempest2/LogicalDbBatchTest.kt +++ b/tempest2/src/test/kotlin/app/cash/tempest2/LogicalDbBatchTest.kt @@ -86,6 +86,33 @@ class LogicalDbBatchTest { assertThat(loadedItems.getItems()).containsExactly(playlistInfo) } + @Test + fun `batchLoad more than 16MB`() = runBlockingTest { + val threeHundredKbName = "a".repeat(300_000) + + // Generate ~30MB of data. Dynamo only supports reading 16MB per batch request, so we need to handled retries to + // get the second page of data + val albumTracks = (1 until (MAX_BATCH_READ)).map { + AlbumTrack("ALBUM_1", it.toLong(), threeHundredKbName, Duration.parse("PT3M28S")) + } + for (albumTrack in albumTracks) { + musicTable.albumTracks.save(albumTrack) + } + val playlistInfo = PlaylistInfo( + playlist_token = "PLAYLIST_1", + playlist_name = "WFH Music", + playlist_tracks = listOf(AlbumTrack.Key("ALBUM_1", 1)) + ) + musicTable.playlistInfo.save(playlistInfo) + + val loadedItems = musicDb.batchLoad( + PlaylistInfo.Key("PLAYLIST_1"), + albumTracks.map { AlbumTrack.Key("ALBUM_1", track_number = it.track_number) } + ) + assertThat(loadedItems.getItems()).containsExactlyInAnyOrderElementsOf(albumTracks) + assertThat(loadedItems.getItems()).containsExactly(playlistInfo) + } + @Test fun batchLoadMultipleTables() { val albumTracks = listOf( From 8edaa77eb3e0cab8201fcf485aae8461d82264ec Mon Sep 17 00:00:00 2001 From: Felix Jancso-Szabo Date: Tue, 2 Apr 2024 12:12:15 -0400 Subject: [PATCH 3/6] Add trackdescription to avoid LSI test failure issue --- .../kotlin/app/cash/tempest2/musiclibrary/MusicItem.kt | 1 + .../app/cash/tempest2/musiclibrary/MusicTable.kt | 8 +++++--- .../kotlin/app/cash/tempest2/LogicalDbBatchTest.kt | 10 ++++++++-- 3 files changed, 14 insertions(+), 5 deletions(-) diff --git a/samples/musiclibrary2/src/main/kotlin/app/cash/tempest2/musiclibrary/MusicItem.kt b/samples/musiclibrary2/src/main/kotlin/app/cash/tempest2/musiclibrary/MusicItem.kt index 11ca61bd..c6528928 100644 --- a/samples/musiclibrary2/src/main/kotlin/app/cash/tempest2/musiclibrary/MusicItem.kt +++ b/samples/musiclibrary2/src/main/kotlin/app/cash/tempest2/musiclibrary/MusicItem.kt @@ -54,6 +54,7 @@ class MusicItem { var track_title: String? = null @get:DynamoDbConvertedBy(DurationTypeConverter::class) var run_length: Duration? = null + var track_description: String? = null // PlaylistInfo. var playlist_name: String? = null diff --git a/samples/musiclibrary2/src/main/kotlin/app/cash/tempest2/musiclibrary/MusicTable.kt b/samples/musiclibrary2/src/main/kotlin/app/cash/tempest2/musiclibrary/MusicTable.kt index 243ae742..eaf9bff7 100644 --- a/samples/musiclibrary2/src/main/kotlin/app/cash/tempest2/musiclibrary/MusicTable.kt +++ b/samples/musiclibrary2/src/main/kotlin/app/cash/tempest2/musiclibrary/MusicTable.kt @@ -94,14 +94,16 @@ data class AlbumTrack( @Attribute(name = "sort_key", prefix = "TRACK_") val track_token: String, val track_title: String, - val run_length: Duration + val run_length: Duration, + val track_description: String = "", ) { constructor( album_token: String, track_number: Long, track_title: String, - run_length: Duration - ) : this(album_token, "%016x".format(track_number), track_title, run_length) + run_length: Duration, + track_description: String = "", + ) : this(album_token, "%016x".format(track_number), track_title, run_length, track_description) @Transient val key = Key(album_token, track_token) diff --git a/tempest2/src/test/kotlin/app/cash/tempest2/LogicalDbBatchTest.kt b/tempest2/src/test/kotlin/app/cash/tempest2/LogicalDbBatchTest.kt index 04583762..d6e06419 100644 --- a/tempest2/src/test/kotlin/app/cash/tempest2/LogicalDbBatchTest.kt +++ b/tempest2/src/test/kotlin/app/cash/tempest2/LogicalDbBatchTest.kt @@ -88,12 +88,18 @@ class LogicalDbBatchTest { @Test fun `batchLoad more than 16MB`() = runBlockingTest { - val threeHundredKbName = "a".repeat(300_000) + val threeHundredKbDescription = "a".repeat(300_000) // Generate ~30MB of data. Dynamo only supports reading 16MB per batch request, so we need to handled retries to // get the second page of data val albumTracks = (1 until (MAX_BATCH_READ)).map { - AlbumTrack("ALBUM_1", it.toLong(), threeHundredKbName, Duration.parse("PT3M28S")) + AlbumTrack( + "ALBUM_1", + it.toLong(), + "track $it", + Duration.parse("PT3M28S"), + threeHundredKbDescription, + ) } for (albumTrack in albumTracks) { musicTable.albumTracks.save(albumTrack) From f1fa4e2993ccfebe6c529d50dd7eb0c64ecb708c Mon Sep 17 00:00:00 2001 From: Felix Jancso-Szabo Date: Tue, 2 Apr 2024 13:38:25 -0400 Subject: [PATCH 4/6] Drop to 200kb --- .../kotlin/app/cash/tempest2/AsyncLogicalDbBatchTest.kt | 1 - .../src/test/kotlin/app/cash/tempest2/LogicalDbBatchTest.kt | 6 +++--- .../src/test/kotlin/app/cash/tempest2/WritingPagerTest.kt | 2 -- 3 files changed, 3 insertions(+), 6 deletions(-) diff --git a/tempest2/src/test/kotlin/app/cash/tempest2/AsyncLogicalDbBatchTest.kt b/tempest2/src/test/kotlin/app/cash/tempest2/AsyncLogicalDbBatchTest.kt index f1708a04..e987510f 100644 --- a/tempest2/src/test/kotlin/app/cash/tempest2/AsyncLogicalDbBatchTest.kt +++ b/tempest2/src/test/kotlin/app/cash/tempest2/AsyncLogicalDbBatchTest.kt @@ -23,7 +23,6 @@ import app.cash.tempest2.musiclibrary.testDb import app.cash.tempest2.testing.asyncLogicalDb import org.assertj.core.api.Assertions.assertThat import org.junit.jupiter.api.Test -import org.junit.jupiter.api.Disabled import org.junit.jupiter.api.extension.RegisterExtension import java.time.Duration diff --git a/tempest2/src/test/kotlin/app/cash/tempest2/LogicalDbBatchTest.kt b/tempest2/src/test/kotlin/app/cash/tempest2/LogicalDbBatchTest.kt index d6e06419..876f56ed 100644 --- a/tempest2/src/test/kotlin/app/cash/tempest2/LogicalDbBatchTest.kt +++ b/tempest2/src/test/kotlin/app/cash/tempest2/LogicalDbBatchTest.kt @@ -88,9 +88,9 @@ class LogicalDbBatchTest { @Test fun `batchLoad more than 16MB`() = runBlockingTest { - val threeHundredKbDescription = "a".repeat(300_000) + val twoHundredKbDescription = "a".repeat(200_000) - // Generate ~30MB of data. Dynamo only supports reading 16MB per batch request, so we need to handled retries to + // Generate ~20MB of data. Dynamo only supports reading 16MB per batch request, so we need to handled retries to // get the second page of data val albumTracks = (1 until (MAX_BATCH_READ)).map { AlbumTrack( @@ -98,7 +98,7 @@ class LogicalDbBatchTest { it.toLong(), "track $it", Duration.parse("PT3M28S"), - threeHundredKbDescription, + twoHundredKbDescription, ) } for (albumTrack in albumTracks) { diff --git a/tempest2/src/test/kotlin/app/cash/tempest2/WritingPagerTest.kt b/tempest2/src/test/kotlin/app/cash/tempest2/WritingPagerTest.kt index b587b943..86b56e8a 100644 --- a/tempest2/src/test/kotlin/app/cash/tempest2/WritingPagerTest.kt +++ b/tempest2/src/test/kotlin/app/cash/tempest2/WritingPagerTest.kt @@ -31,8 +31,6 @@ import org.junit.jupiter.api.extension.RegisterExtension import software.amazon.awssdk.enhanced.dynamodb.Expression import software.amazon.awssdk.services.dynamodb.model.AttributeValue import software.amazon.awssdk.services.dynamodb.model.TransactionCanceledException -import java.util.function.BiFunction -import kotlin.streams.toList class WritingPagerTest { From e5cb17227ec32270dee1cef5ed54bcf342b4b4f8 Mon Sep 17 00:00:00 2001 From: Felix Jancso-Szabo Date: Tue, 2 Apr 2024 13:41:11 -0400 Subject: [PATCH 5/6] Fix strange ArrayList bug --- .../src/test/kotlin/app/cash/tempest2/LogicalDbBatchTest.kt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tempest2/src/test/kotlin/app/cash/tempest2/LogicalDbBatchTest.kt b/tempest2/src/test/kotlin/app/cash/tempest2/LogicalDbBatchTest.kt index 876f56ed..34ee6422 100644 --- a/tempest2/src/test/kotlin/app/cash/tempest2/LogicalDbBatchTest.kt +++ b/tempest2/src/test/kotlin/app/cash/tempest2/LogicalDbBatchTest.kt @@ -113,7 +113,7 @@ class LogicalDbBatchTest { val loadedItems = musicDb.batchLoad( PlaylistInfo.Key("PLAYLIST_1"), - albumTracks.map { AlbumTrack.Key("ALBUM_1", track_number = it.track_number) } + *(albumTracks.map { AlbumTrack.Key("ALBUM_1", track_number = it.track_number) }.toTypedArray()) ) assertThat(loadedItems.getItems()).containsExactlyInAnyOrderElementsOf(albumTracks) assertThat(loadedItems.getItems()).containsExactly(playlistInfo) From 44e92dfff976e0a8b687a95debb8200f5f48c12f Mon Sep 17 00:00:00 2001 From: Felix Jancso-Szabo Date: Tue, 2 Apr 2024 13:42:28 -0400 Subject: [PATCH 6/6] Fix SchemaTest --- .../src/test/kotlin/app/cash/tempest2/internal/SchemaTest.kt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tempest2/src/test/kotlin/app/cash/tempest2/internal/SchemaTest.kt b/tempest2/src/test/kotlin/app/cash/tempest2/internal/SchemaTest.kt index e5ab5cfa..8feb18c1 100644 --- a/tempest2/src/test/kotlin/app/cash/tempest2/internal/SchemaTest.kt +++ b/tempest2/src/test/kotlin/app/cash/tempest2/internal/SchemaTest.kt @@ -86,7 +86,7 @@ class SchemaTest { assertThatIllegalArgumentException().isThrownBy { musicDb.music.inlineView(Any::class, BadItem5::class) - }.withMessage("Expect nonexistent_attribute, required by class app.cash.tempest2.internal.BadItem5, to be declared in class app.cash.tempest2.musiclibrary.MusicItem. But found [album_title, artist_name, genre_name, label_name, partition_key, playlist_name, playlist_size, playlist_tracks, playlist_version, release_date, run_length, sort_key, track_title, track_token]. Use @Transient to exclude it. You might see this error message if the property name starts with `is`. See: https://github.com/cashapp/tempest/issues/53.") + }.withMessage("Expect nonexistent_attribute, required by class app.cash.tempest2.internal.BadItem5, to be declared in class app.cash.tempest2.musiclibrary.MusicItem. But found [album_title, artist_name, genre_name, label_name, partition_key, playlist_name, playlist_size, playlist_tracks, playlist_version, release_date, run_length, sort_key, track_description, track_title, track_token]. Use @Transient to exclude it. You might see this error message if the property name starts with `is`. See: https://github.com/cashapp/tempest/issues/53.") assertThatIllegalArgumentException().isThrownBy { musicDb.music.inlineView(Any::class, BadItem6::class)