nsivabalan commented on code in PR #18073:
URL: https://github.com/apache/hudi/pull/18073#discussion_r2796865956


##########
hudi-timeline-service/src/main/java/org/apache/hudi/timeline/service/handlers/marker/MarkerDirState.java:
##########
@@ -80,8 +83,9 @@ public class MarkerDirState implements Serializable {
   // {@code true} means the file is in use by a {@code 
BatchCreateMarkerRunnable}.
   // Index of the list is used for the filename, i.e., "1" -> "MARKERS1"
   private final List<Boolean> threadUseStatus;
-  // A list of pending futures from async marker creation requests
-  private final List<MarkerCreationFuture> markerCreationFutures = new 
ArrayList<>();
+  // Map of in-flight requests: (markerName + "|" + requestId) -> 
MarkerCreationFuture
+  // Used for BOTH deduplication AND batch marker creation requests queue
+  private final Map<String, MarkerCreationFuture> inflightRequestMap = new 
ConcurrentHashMap<>();

Review Comment:
   lets use a Pair<String, String> instead of using some encoding



##########
hudi-timeline-service/src/main/java/org/apache/hudi/timeline/service/handlers/marker/MarkerDirState.java:
##########
@@ -265,13 +286,14 @@ public void processMarkerCreationRequests(
    */
   public boolean deleteAllMarkers() {
     boolean result = FSUtils.deleteDir(hoodieEngineContext, storage, 
markerDirPath, parallelism);
-    allMarkers.clear();
+    markerToRequestIdMap.clear();
     fileMarkersMap.clear();
     return result;
   }
 
   /**
    * Syncs all markers maintained in the underlying files under the marker 
directory in the file system.
+   * When TLS recovers from crash, we have no request IDs so store sentinel 
value for each marker.

Review Comment:
   looks like from the latest state of the patch, this is what I see. 
   
   any incoming requests has to have non null request Id. 
   and markerToRequestIdMap will be containing valid request Ids as values if 
not for crash scenarios. 
   
   so that, we can return success for a retry from executor from the same task. 
   
   once we resume from crash, the values in the map will be `NULL_REQUEST_ID` 
which is empty string. So, if we were to receive a request for an already seen 
marker name, we will fail as the request id will not match. 
   
   can you help us understand whats the mis alignment. we are not changing any 
behaviors, just adding support to let tls request retry succeed if triggered 
using same request id.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to