Browse Source

fix: do not leave the SD.cpp server holding nothing when a load fails

Correcting something I had wrong: the server never unloads a model on its
own. So when it was found empty, that was this node's doing.

Confirmed against the running server: a load while a model is resident is
refused with 409 "call POST /models/unload first", and the resident model
survives the refusal. The unload is therefore genuinely required, and it
opens a window - between emptying the slot and the weights being resident -
where a failure leaves the server holding nothing at all. That is exactly
what the reported execution did: the unload succeeded, the load fell over on
a health read, and every later run then had to load from cold.

A load that fails after the slot was emptied is now retried once. There is
nothing left to lose at that point, the usual cause is a moment of slowness,
and the alternative is leaving the server worse than it was found. If the
second attempt fails too, the error says so outright - that the server is
holding no model and nothing will generate until a load succeeds - instead
of a bare timeout that leaves the state to be discovered later.

The comment above the unload now records why it is there, so the next reader
does not remove it on the strength of the documentation, and what it costs.

Unchanged, and confirmed still true: a model already loaded with the wanted
settings is left alone. Second run in a row took 10ms and never touched the
server.

63 passed.
fszontagh 1 tháng trước cách đây
mục cha
commit
0aa2587d9e
1 tập tin đã thay đổi với 41 bổ sung và 3 xóa
  1. 41 3
      nodes/sdcpp/sdcpp-model-load.js

+ 41 - 3
nodes/sdcpp/sdcpp-model-load.js

@@ -561,12 +561,20 @@ async function execute(config, input, context) {
         (current ? ' (replacing ' + current + ')' : ''));
         (current ? ' (replacing ' + current + ')' : ''));
 
 
     // The slot has to be emptied first. The API documentation says a load
     // The slot has to be emptied first. The API documentation says a load
-    // replaces whatever is there, but the server answers "A model is already
-    // loaded. Call POST /models/unload first" - so it is unloaded here rather
-    // than leaving every reload to fail on a server that already has a model.
+    // replaces whatever is there, but the server answers 409 "A model is
+    // already loaded. Call POST /models/unload first" - so it is unloaded here
+    // rather than leaving every reload to fail on a server that already has a
+    // model. (A refused load is harmless: the resident model stays put.)
     //
     //
     // This is also the only way to change the settings of a model that is
     // This is also the only way to change the settings of a model that is
     // already loaded, which is the case this node exists to handle.
     // already loaded, which is the case this node exists to handle.
+    //
+    // It does mean everything between here and a finished load runs with the
+    // server holding nothing. The server never unloads on its own, so an empty
+    // slot afterwards is always something that happened in this window - which
+    // is why the failure paths below say so rather than leaving the next run to
+    // discover it.
+    let emptiedTheSlot = false;
     if (health.model_loaded === true) {
     if (health.model_loaded === true) {
         call({
         call({
             method: 'POST',
             method: 'POST',
@@ -577,6 +585,7 @@ async function execute(config, input, context) {
             what: 'unloading ' + (current || 'the current model') + ' before loading ' + modelName
             what: 'unloading ' + (current || 'the current model') + ' before loading ' + modelName
         });
         });
         smartbotic.log.info('SD.cpp: unloaded ' + (current || 'the previous model'));
         smartbotic.log.info('SD.cpp: unloaded ' + (current || 'the previous model'));
+        emptiedTheSlot = true;
     }
     }
 
 
     // Loading unloads whatever was in the slot first, and the server holds a
     // Loading unloads whatever was in the slot first, and the server holds a
@@ -601,6 +610,35 @@ async function execute(config, input, context) {
         // just impatience.
         // just impatience.
         const probe = pollHealth(server, 15000);
         const probe = pollHealth(server, 15000);
         if (!probe || probe.model_loading !== true) {
         if (!probe || probe.model_loading !== true) {
+            // The slot was emptied to make room and the load did not take, so
+            // the server now holds nothing. One more attempt is worth it: there
+            // is nothing left to lose, the usual cause is a moment of
+            // slowness, and the alternative is leaving the server worse than it
+            // was found.
+            if (emptiedTheSlot && (!probe || probe.model_loaded !== true)) {
+                smartbotic.log.warn('SD.cpp: the load failed and the server now has no model. ' +
+                    'Trying once more before giving up');
+                try {
+                    loaded = call({
+                        method: 'POST',
+                        url: server + '/models/load',
+                        headers: { 'Content-Type': 'application/json',
+                                   'Authorization': 'Bearer ' + token },
+                        body: JSON.stringify(body),
+                        timeout: timeout,
+                        what: 'loading model ' + modelName + ' (second attempt)'
+                    });
+                    // Falls through to the wait below, the same as a first
+                    // attempt that worked - the model still has to finish
+                    // loading either way.
+                } catch (secondError) {
+                    throw new Error('SD.cpp: could not load ' + modelName + ', and the server ' +
+                        'is now holding no model at all - it was unloaded to make room. ' +
+                        'Nothing will generate until a load succeeds. First attempt: ' +
+                        ((loadError && loadError.message) || loadError) + '. Second: ' +
+                        ((secondError && secondError.message) || secondError));
+                }
+            }
             throw loadError;
             throw loadError;
         }
         }
         smartbotic.log.info('SD.cpp: the load request stopped waiting, but the server is still ' +
         smartbotic.log.info('SD.cpp: the load request stopped waiting, but the server is still ' +