Skip to content

Enhancement: Check ComfyUI server cache before image upload#2468

Open
rbeurskens wants to merge 2 commits intoAcly:mainfrom
rbeurskens:enhancement/check-server-cache-before-image-upload
Open

Enhancement: Check ComfyUI server cache before image upload#2468
rbeurskens wants to merge 2 commits intoAcly:mainfrom
rbeurskens:enhancement/check-server-cache-before-image-upload

Conversation

@rbeurskens
Copy link
Copy Markdown

@rbeurskens rbeurskens commented Apr 27, 2026

Should solve #2467
Also saves some network traffic by not sending the payload if HEAD returned 200. (in contrast to PUT)

… if the server closes the connection on a cache hit before the client sends the PUT payload.
@Acly
Copy link
Copy Markdown
Owner

Acly commented May 1, 2026

I've never encountered the "Unable to write" error before, does that happen all the time for you or with particular image size? Which OS?

I don't think an additional HEAD request is a necessarily good trade of, now every upload needs an extra round trip, and the not-cached case is probably far more common.

I consider sending some of the upload data even if it's cached okay, but it would require a way to selectively ignore this particular error, which might be difficult if Qt doesn't return the actual response and 200 status code. Since I can't reproduce it, it's hard to tell.

Cleaner solution might be to start the PUT with a Expect: 100-continue header which is less overhead than the HEAD request, but I haven't tested it yet.

@rbeurskens
Copy link
Copy Markdown
Author

rbeurskens commented May 1, 2026

For me, consistent from consecutive prompts using one or more of the same source image(s) when using a custom ComfyUI server (latest) running on Kubuntu LTS 24.04 on a 2012 era PC (MSI Z77 GD65 mainboard, i7 3770K, 16GB). Krita running on Windows 10 12gen i7,16GB.
I will test your suggestion and let you know if that fixes it too.
The issue could be caused by the combination of the modern Linux running on 2012 hardware (in particular when it comes to power management of the onboard ethernet adapter)

Images were not really large (this one was ~1MB )

@Acly
Copy link
Copy Markdown
Owner

Acly commented May 1, 2026

Might just have to do with latency, I mostly test local with Krita & Comfy running on the same machine.

Note that the 100-continue solution would require support by the server too (ie. in comfyui-tooling-nodes)

@rbeurskens
Copy link
Copy Markdown
Author

rbeurskens commented May 1, 2026

Yes, probably. I never had this when running the local managed server. After all, the loopback likely doesn't have this issue. But with an actual remote it's different. (maybe even worse with a cloud server instead of one in the LAN)
I have the latest comfyUI-tooling-nodes node cloned on the server. As far as I know the header is a high level protocol feature that does not need hardware/driver level support to work, as long as the HTTP listener supports it. (aiohttp)

The problem is that, as you also said, QtReply will only process response data if it sent the full payload successfully. No matter if the PUT is sent with Expect-100 or without, if the server sees it has the image cached before the client has finished sending the payload, it will send the 200 OK response with the { "status" : "cached" } json and close the connection immediately. This will cause QNetworkReply::RemoteHostClosedError.
If the lightweight HEAD it too much, we would need to handle the request on a lower level and handle the request manually using QTcpSocket, and keep monitoring server response for 200 OK and stop once it's found to prevent the error QNetworkAccessManager would trigger.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants