@@ -60,9 +60,9 @@ public class RecallEngineClient {
6060 private final List <WriteItem > writeData = new ArrayList <>();
6161 private final ReentrantLock writeLock = new ReentrantLock ();
6262 private final Condition writeCondition = writeLock .newCondition ();
63- private ExecutorService writeExecutor ;
63+ private volatile ExecutorService writeExecutor ;
6464 private volatile boolean running = true ;
65- private Thread asyncWriteThread ;
65+ private volatile Thread asyncWriteThread ;
6666 private int batchSize = 20 ;
6767 private long flushIntervalMs = 50 ;
6868 private WriteCallback globalCallback ;
@@ -278,7 +278,7 @@ public WriteResponse write(String instanceId, String table, WriteRequest request
278278 }
279279
280280 int itemCount = request .getContent ().size ();
281- String insertMode = request .getInsertMode ();
281+ InsertMode insertMode = request .getInsertMode ();
282282
283283 writeLock .lock ();
284284 try {
@@ -324,7 +324,7 @@ public WriteResponse write(String instanceId, String table, WriteRequest request
324324 }
325325
326326 int itemCount = request .getContent ().size ();
327- String insertMode = request .getInsertMode ();
327+ InsertMode insertMode = request .getInsertMode ();
328328
329329 writeLock .lock ();
330330 try {
@@ -471,52 +471,85 @@ public RecallEngineClient withWriteThreadPoolSize(int poolSize) {
471471 }
472472
473473 /**
474- * Get or create the write executor
474+ * Get or create the write executor using Double-Checked Locking for thread safety and performance.
475+ * This pattern ensures:
476+ * - Thread safety: only one thread creates the executor
477+ * - Performance: subsequent calls only need a volatile read (no synchronization)
475478 */
476479 private ExecutorService getWriteExecutor () {
477- if (writeExecutor == null || writeExecutor .isShutdown ()) {
478- writeExecutor = Executors .newFixedThreadPool (writeThreadPoolSize );
480+ ExecutorService executor = writeExecutor ;
481+ if (executor == null || executor .isShutdown ()) {
482+ synchronized (this ) {
483+ executor = writeExecutor ;
484+ if (executor == null || executor .isShutdown ()) {
485+ executor = Executors .newFixedThreadPool (writeThreadPoolSize );
486+ writeExecutor = executor ;
487+ }
488+ }
479489 }
480- return writeExecutor ;
490+ return executor ;
481491 }
482492
483493 /**
484- * Start the async write background thread
494+ * Start the async write background thread.
495+ * Uses Double-Checked Locking for thread safety and performance:
496+ * - First check without lock (fast path for most calls)
497+ * - Only acquire lock when thread needs to be created
485498 */
486499 private void startAsyncWriteThread () {
487- if (asyncWriteThread != null && asyncWriteThread .isAlive ()) {
500+ // Fast path: if thread is already running, return immediately (no lock needed)
501+ Thread thread = asyncWriteThread ;
502+ if (thread != null && thread .isAlive ()) {
488503 return ;
489504 }
490505
491- String threadName = "RecallEngineAsyncWriter" ;
492- asyncWriteThread = new Thread (() -> {
493- while (running ) {
506+ // Slow path: need to create thread, use synchronized
507+ synchronized (this ) {
508+ // Double-check after acquiring lock
509+ thread = asyncWriteThread ;
510+ if (thread != null && thread .isAlive ()) {
511+ return ;
512+ }
513+
514+ String threadName = "RecallEngineAsyncWriter" ;
515+ asyncWriteThread = new Thread (() -> {
516+ while (running ) {
517+ writeLock .lock ();
518+ try {
519+ writeCondition .await (flushIntervalMs , TimeUnit .MILLISECONDS );
520+ if (!writeData .isEmpty ()) {
521+ doAsyncWrite ();
522+ }
523+ } catch (InterruptedException e ) {
524+ logger .warn ("{} interrupted, flushing remaining data before exit" , threadName );
525+ Thread .currentThread ().interrupt ();
526+ // Flush remaining data before exit
527+ try {
528+ if (!writeData .isEmpty ()) {
529+ doAsyncWrite ();
530+ }
531+ } catch (Exception ex ) {
532+ logger .error ("Failed to flush data on interrupt" , ex );
533+ }
534+ break ;
535+ } finally {
536+ writeLock .unlock ();
537+ }
538+ }
539+ // Handle remaining data after thread stops
494540 writeLock .lock ();
495541 try {
496- writeCondition .await (flushIntervalMs , TimeUnit .MILLISECONDS );
497542 if (!writeData .isEmpty ()) {
498543 doAsyncWrite ();
499544 }
500- } catch (InterruptedException e ) {
501- Thread .currentThread ().interrupt ();
502- break ;
503545 } finally {
504546 writeLock .unlock ();
505547 }
506- }
507- // Handle remaining data after thread stops
508- writeLock .lock ();
509- try {
510- if (!writeData .isEmpty ()) {
511- doAsyncWrite ();
512- }
513- } finally {
514- writeLock .unlock ();
515- }
516- logger .info (threadName + " has stopped." );
517- }, threadName );
518- asyncWriteThread .setDaemon (true );
519- asyncWriteThread .start ();
548+ logger .info ("{} has stopped." , threadName );
549+ }, threadName );
550+ asyncWriteThread .setDaemon (true );
551+ asyncWriteThread .start ();
552+ }
520553 }
521554
522555 /**
@@ -555,19 +588,19 @@ private Future<?> doAsyncWrite() {
555588 writeData .clear ();
556589
557590 return getWriteExecutor ().submit (() -> {
558- // Group by instanceId, table, callback, and insertMode
559- Map <String , List <WriteItem >> groupedItems = new HashMap <>();
591+ // Group by instanceId, table, callback, and insertMode using a composite key object
592+ Map <GroupKey , List <WriteItem >> groupedItems = new HashMap <>();
560593 for (WriteItem item : tempList ) {
561- // Include callback in the key to group items with same callback together
562- String key = item . instanceId + ":" + item . table + ":" + System .identityHashCode (item .callback )+ ":" + item .insertMode ;
594+ GroupKey key = new GroupKey ( item . instanceId , item . table ,
595+ System .identityHashCode (item .callback ), item .insertMode ) ;
563596 groupedItems .computeIfAbsent (key , k -> new ArrayList <>()).add (item );
564597 }
565598
566599 // Write each group
567- for (Map .Entry <String , List <WriteItem >> entry : groupedItems .entrySet ()) {
568- String [] parts = entry .getKey (). split ( ":" , 3 );
569- String instanceId = parts [ 0 ] ;
570- String table = parts [ 1 ] ;
600+ for (Map .Entry <GroupKey , List <WriteItem >> entry : groupedItems .entrySet ()) {
601+ GroupKey key = entry .getKey ();
602+ String instanceId = key . instanceId ;
603+ String table = key . table ;
571604 List <WriteItem > items = entry .getValue ();
572605
573606 // Get callback from first item (all items in group have same callback)
@@ -594,18 +627,18 @@ private Future<?> doAsyncWrite() {
594627 try {
595628 callback .onSuccess (instanceId , table , response );
596629 } catch (Exception e ) {
597- logger .error ("Callback onError raised exception: {} " , e . getMessage () );
630+ logger .error ("Callback onSuccess raised exception" , e );
598631 }
599632 }
600633 } catch (Exception e ) {
601- logger .error ("Async write failed for {}/{}: {}" , instanceId , table , e .getMessage ());
634+ logger .error ("Async write failed for {}/{}: {}" , instanceId , table , e .getMessage (), e );
602635
603636 // Invoke error callback
604637 if (callback != null ) {
605638 try {
606639 callback .onError (instanceId , table , e );
607640 } catch (Exception ex ) {
608- logger .error ("Callback onError raised exception: {} " , ex . getMessage () );
641+ logger .error ("Callback onError raised exception" , ex );
609642 }
610643 }
611644 }
@@ -659,25 +692,49 @@ private static class WriteItem {
659692 final String instanceId ;
660693 final String table ;
661694 final Map <String , Object > data ;
662- final long timestamp ;
663695 final WriteCallback callback ;
664- final String insertMode ;
696+ final InsertMode insertMode ;
665697
666- WriteItem (String instanceId , String table , Map <String , Object > data ) {
667- this (instanceId , table , data , null , null );
698+ WriteItem (String instanceId , String table , Map <String , Object > data , WriteCallback callback , InsertMode insertMode ) {
699+ this .instanceId = instanceId ;
700+ this .table = table ;
701+ this .data = data ;
702+ this .callback = callback ;
703+ this .insertMode = insertMode ;
668704 }
705+ }
669706
670- WriteItem (String instanceId , String table , Map <String , Object > data , WriteCallback callback ) {
671- this (instanceId , table , data , callback , null );
672- }
707+ /**
708+ * Internal class for grouping write items by instanceId, table, callback, and insertMode.
709+ * Using a proper key object avoids string parsing issues when instanceId or table contains ":".
710+ */
711+ private static class GroupKey {
712+ final String instanceId ;
713+ final String table ;
714+ final int callbackHash ;
715+ final InsertMode insertMode ;
673716
674- WriteItem (String instanceId , String table , Map < String , Object > data , WriteCallback callback , String insertMode ) {
717+ GroupKey (String instanceId , String table , int callbackHash , InsertMode insertMode ) {
675718 this .instanceId = instanceId ;
676719 this .table = table ;
677- this .data = data ;
678- this .timestamp = System .currentTimeMillis ();
679- this .callback = callback ;
720+ this .callbackHash = callbackHash ;
680721 this .insertMode = insertMode ;
681722 }
723+
724+ @ Override
725+ public boolean equals (Object o ) {
726+ if (this == o ) return true ;
727+ if (o == null || getClass () != o .getClass ()) return false ;
728+ GroupKey groupKey = (GroupKey ) o ;
729+ return callbackHash == groupKey .callbackHash &&
730+ java .util .Objects .equals (instanceId , groupKey .instanceId ) &&
731+ java .util .Objects .equals (table , groupKey .table ) &&
732+ insertMode == groupKey .insertMode ;
733+ }
734+
735+ @ Override
736+ public int hashCode () {
737+ return java .util .Objects .hash (instanceId , table , callbackHash , insertMode );
738+ }
682739 }
683740}
0 commit comments