Fix cache cleanup for duplicate repacked chunks

Duplicates may come from the storage modules' 100 MiB overlaps.
ArweaveTeam · Oct 23, 2024 · 7ed7395 · 7ed7395
1 parent c14c86d
commit 7ed7395
Show file tree

Hide file tree

Showing 2 changed files with 38 additions and 35 deletions.
diff --git a/apps/arweave/src/ar_data_sync.erl b/apps/arweave/src/ar_data_sync.erl
@@ -2728,7 +2728,7 @@ pack_and_store_chunk({_, AbsoluteOffset, _, _, _, _, _, _, _, _, _, _},
 pack_and_store_chunk(Args, State) ->
 	{DataRoot, AbsoluteOffset, TXPath, TXRoot, DataPath, Packing, Offset, ChunkSize, Chunk,
 			UnpackedChunk, OriginStoreID, OriginChunkDataKey} = Args,
-	#sync_data_state{ packing_map = PackingMap, store_id = StoreID } = State,
+	#sync_data_state{ store_id = StoreID } = State,
 	RequiredPacking = get_required_chunk_packing(AbsoluteOffset, ChunkSize, State),
 	PackingStatus =
 		case {RequiredPacking, Packing} of
@@ -2746,40 +2746,34 @@ pack_and_store_chunk(Args, State) ->
 			gen_server:cast(DataSyncStorage, {store_chunk, ChunkArgs, ExtraArgs}),
 			{noreply, State};
 		{need_packing, RequiredPacking} ->
-			case maps:is_key({AbsoluteOffset, RequiredPacking}, PackingMap) of
+			case ar_packing_server:is_buffer_full() of
 				true ->
-					decrement_chunk_cache_size(StoreID),
+					ar_util:cast_after(1000, self(), {pack_and_store_chunk, Args}),
 					{noreply, State};
 				false ->
-					case ar_packing_server:is_buffer_full() of
-						true ->
-							ar_util:cast_after(1000, self(), {pack_and_store_chunk, Args}),
-							{noreply, State};
-						false ->
-							{Packing2, Chunk2} =
-								case UnpackedChunk of
-									none ->
-										{Packing, Chunk};
-									_ ->
-										{unpacked, UnpackedChunk}
-								end,
-							DataSyncStorage = ar_data_sync_storage:name(StoreID),
-							PromiseKey = {AbsoluteOffset, RequiredPacking},
-							PromiseArgs = {pack_chunk, {RequiredPacking, DataPath,
-									Offset, DataRoot, TXPath, OriginStoreID,
-									OriginChunkDataKey}},
-							gen_server:cast(DataSyncStorage,
-									{packed_chunk_promise, PromiseKey, PromiseArgs}),
-							RepackArgs = {RequiredPacking, Packing2, Chunk2, AbsoluteOffset,
-									TXRoot, ChunkSize},
-							ar_packing_server:request_repack(
-									AbsoluteOffset,
-									whereis(DataSyncStorage),
-									RepackArgs),
-							ar_util:cast_after(600000, DataSyncStorage,
-									{expire_repack_chunk_request, PromiseKey}),
-							{noreply, State}
-					end
+					{Packing2, Chunk2} =
+						case UnpackedChunk of
+							none ->
+								{Packing, Chunk};
+							_ ->
+								{unpacked, UnpackedChunk}
+						end,
+					DataSyncStorage = ar_data_sync_storage:name(StoreID),
+					PromiseKey = {AbsoluteOffset, RequiredPacking},
+					PromiseArgs = {pack_chunk, {RequiredPacking, DataPath,
+							Offset, DataRoot, TXPath, OriginStoreID,
+							OriginChunkDataKey}},
+					gen_server:cast(DataSyncStorage,
+							{packed_chunk_promise, PromiseKey, PromiseArgs}),
+					RepackArgs = {RequiredPacking, Packing2, Chunk2, AbsoluteOffset,
+							TXRoot, ChunkSize},
+					ar_packing_server:request_repack(
+							AbsoluteOffset,
+							whereis(DataSyncStorage),
+							RepackArgs),
+					ar_util:cast_after(600000, DataSyncStorage,
+							{expire_repack_chunk_request, PromiseKey}),
+					{noreply, State}
 			end
 	end.
 

diff --git a/apps/arweave/src/ar_data_sync_storage.erl b/apps/arweave/src/ar_data_sync_storage.erl
@@ -91,9 +91,16 @@ handle_cast({expire_repack_chunk_request, Key}, State) ->
 	end;
 
 handle_cast({packed_chunk_promise, Key, Args}, State) ->
-	#state{ packing_map = Map } = State,
-	Map2 = maps:put(Key, Args, Map),
-	{noreply, State#state{ packing_map = Map2 }};
+	#state{ packing_map = Map, store_id = StoreID } = State,
+	case maps:is_key(Key, Map) of
+		true ->
+			%% Free up the cache for the duplicate.
+			ar_data_sync:decrement_chunk_cache_size(StoreID),
+			{noreply, State};
+		false ->
+			Map2 = maps:put(Key, Args, Map),
+			{noreply, State#state{ packing_map = Map2 }}
+	end;
 
 handle_cast(process_store_chunk_queue, State) ->
 	ar_util:cast_after(200, self(), process_store_chunk_queue),
@@ -119,6 +126,8 @@ handle_info({chunk, {packed, Offset, ChunkArgs}}, State) ->
 			State2 = State#state{ packing_map = maps:remove(Key, PackingMap) },
 			{noreply, store_chunk(ChunkArgs, Args, State2)};
 		_ ->
+			%% Should be a duplicate - no need to do anything as we
+			%% clean up the cache earlier in the packed_chunk_promise handler.
 			{noreply, State}
 	end;