PR feedback.

Signed-off-by: Marc Handalian <handalm@amazon.com>

server/src/main/java/org/opensearch/index/engine/NRTReplicationEngine.java

@@ -133,8 +133,9 @@ public synchronized void updateSegments(final SegmentInfos infos, long seqNo) th
      * Persist the latest live SegmentInfos.
      *
      * This method creates a commit point from the latest SegmentInfos. It is intended to be used when this shard is about to be promoted as the new primary.
-     * If this method is invoked while the engine is currently updating segments on its reader, it will wait for that update to complete so the updated segments are used.
-     * It does not wait for segment copy to complete, that
+     *
+     * TODO: If this method is invoked while the engine is currently updating segments on its reader, wait for that update to complete so the updated segments are used.
+     *
      *
      * @throws IOException - When there is an IO error committing the SegmentInfos.
      */

server/src/main/java/org/opensearch/index/shard/CheckpointRefreshListener.java

@@ -40,8 +40,7 @@ public void beforeRefresh() throws IOException {
 
     @Override
     public void afterRefresh(boolean didRefresh) throws IOException {
-        if (didRefresh && shard.state() != IndexShardState.CLOSED &&
-            shard.getReplicationTracker().isPrimaryMode()) {
+        if (didRefresh && shard.state() != IndexShardState.CLOSED && shard.getReplicationTracker().isPrimaryMode()) {
             publisher.publish(shard);
         }
     }

server/src/main/java/org/opensearch/index/shard/IndexShard.java

@@ -624,6 +624,7 @@ public void updateShardState(
                         try {
                             if (indexSettings.isSegRepEnabled()) {
                                 // this Shard's engine was read only, we need to update its engine before restoring local history from xlog.
+                                assert newRouting.primary() && currentRouting.primary() == false;
                                 promoteNRTReplicaToPrimary();
                             }
                             replicationTracker.activatePrimaryMode(getLocalCheckpoint());
@@ -4140,8 +4141,8 @@ private void promoteNRTReplicaToPrimary() {
                 engine.commitSegmentInfos();
                 resetEngineToGlobalCheckpoint();
             } catch (IOException e) {
-                throw new OpenSearchException("Unable to change engine type, failing", e);
+                throw new EngineException(shardId, "Unable to update  replica to writeable engine, failing shard", e);
             }
-        }, () -> { throw new OpenSearchException("Expected replica engine to be of type NRTReplicationEngine"); });
+        }, () -> { throw new EngineException(shardId, "Expected replica engine to be of type NRTReplicationEngine"); });
     }
 }

server/src/main/java/org/opensearch/index/store/Store.java

@@ -829,8 +829,9 @@ public void commitSegmentInfos(SegmentInfos latestSegmentInfos, long maxSeqNo, l
             // similar to TrimUnsafeCommits, create a commit with an appending IW, this will delete old commits and ensure all files
             // associated with the SegmentInfos.commit are fsynced.
             final List<IndexCommit> existingCommits = DirectoryReader.listCommits(directory);
-            assert existingCommits.isEmpty() == false : "No commits found";
+            assert existingCommits.isEmpty() == false : "Expected at least one commit but none found";
             final IndexCommit lastIndexCommit = existingCommits.get(existingCommits.size() - 1);
+            assert latestSegmentInfos.getSegmentsFileName().equals(lastIndexCommit.getSegmentsFileName());
             try (IndexWriter writer = newAppendingIndexWriter(directory, lastIndexCommit)) {
                 writer.setLiveCommitData(lastIndexCommit.getUserData().entrySet());
                 writer.commit();