python/etl: classify direct-put 200 responses by Content-Length#334
python/etl: classify direct-put 200 responses by Content-Length#334chanu1406 wants to merge 1 commit into
Conversation
Signed-off-by: Chanu Ollala <chanuollala@gmail.com>
| return resp.status_code, resp.content, 0 | ||
|
|
||
| return STATUS_NO_CONTENT, b"", size # from target, no content | ||
| # Keyed on the Content-Length header, mirroring the Go webserver's |
There was a problem hiding this comment.
This fixes the original issue for a 2-stage same-target pipeline, but not for 3+ stages.
The final streaming ETL can return 200 with an empty chunked body and no Content-Length, meaning the transformed object is valid but empty. The previous ETL now recognizes that correctly. However, when it forwards the result, the Python server turns it into a buffered empty response with Content-Length: 0. If there is another ETL upstream, that ETL sees Content-Length: 0, assumes the object was already direct-put to the target, and returns 204. In the same-target case no direct put actually happened, so AIS skips writing the empty object.
With only 2 stages, the rewritten 200 goes directly back to AIS, which accepts it as an empty object, so that case works. This is a partial fix to an existing bug, not a new regression. We still need to preserve the unknown/chunked-length state when forwarding the empty response and add a 3-stage test.
This specifically affects same-target pipelines, where the source and destination resolve to the same AIS target. In that case no final direct PUT occurs, so the incorrect 204 causes AIS to skip the actual write.
can you please debug this @chanu1406, its slightly more complex that it seems. We are mixing streaming + pipeline + direct put which is creating a complex situation. If possible add a test but its difficult because how would you know before hand whether on which target the object will be stored. Lmk if you need help
|
@gaikwadabhishek Thanks for the review, this is a valuable finding can you let me know what you think of this approach for the python servers: The issue is the middle stage classifies the empty chunked 200 correctly, but then re-emits it as a buffered empty response. All three Python servers stamp Approach: for transform-content responses, the Python servers should never emit
This keeps For coverage, |
|
@Nahemah1022 thoughts? |
|
@gaikwadabhishek @Nahemah1022 I looked into this issue a bit further, and had some findings. The problem with my earlier idea is that whether a response is chunked or has However the stage already knows what it just sent the PUT to. ETL pipeline entries are host-only URLs, while the destination target URL carries the object path —
Still the same 3 stage regression test plan as before. The failing to_target tests pass again without touching their mocks, and I'll switch the ETL hop URLs in the existing tests to host-only so they match what AIS actually sends. The Go webserver has the same issue on its side, can follow up on that as well. Let me know what you think |
|
I think the core problem is the ambiguity around whether a @chanu1406 You’re right that inspecting the URL could help distinguish between them, but I’m concerned that this would introduce another branch of logic to maintain on top of the already complicated pipeline logic. I wonder whether it would make our lives easier if we updated the AIS target to return This seems like it could remove the ambiguity entirely: ETL responses would always use This is just an idea. @chanu1406 Could you confirm whether this approach would work better based on your understanding of the core issue? |
|
@Nahemah1022 After looking into it, I agree this is a cleaner approach and it should work well. The target currently returns the 200 implicitly after a successful The target would return However, while tracing, I found that there may be a compatibility issue. A webserver using the new semantics but talking to an older target would treat the target’s legacy bare 200 acknowledgment as empty ETL output. In a different-target pipeline, AIS would then send that empty result to the destination and overwrite the object that was just direct-put. To avoid that, either all targets must be upgraded before deploying webservers with the new behavior, or we keep a |
Follow up to #330
Fix Python ETL direct-put handling to match the Go webserver behavior from #330.
I have also added unit coverage for the direct-put response cases, and updated the outdated comment in the Go webserver