Skip to content

Commit

Permalink
Fix an IFRT Proxy version compatibility bug introduced by `ExecuteOpt…
Browse files Browse the repository at this point in the history
…ions::fill_status`

Since `fill_status` was implicitly true before the field was introduced, we should have overridden the `fill_status` from the deserialized options. Without that, the client->server behaves incorrectly when the client is old (doesn't set `fill_status` but expects the future to be populated by the server) but the server is new (has `fill_status` in the proto and assumes that the client set it to false).

PiperOrigin-RevId: 681247988
  • Loading branch information
junwhanahn authored and Google-ML-Automation committed Oct 2, 2024
1 parent ee96be1 commit a4cb02b
Showing 1 changed file with 6 additions and 0 deletions.
6 changes: 6 additions & 0 deletions xla/python/ifrt_proxy/server/ifrt_backend.cc
Original file line number Diff line number Diff line change
Expand Up @@ -1058,6 +1058,12 @@ IfrtBackend::HandleLoadedExecutableExecuteRequest(
TF_ASSIGN_OR_RETURN(auto execute_options,
xla::ifrt::LoadedExecutable::ExecuteOptions::FromProto(
execute.execute_options()));
// Force the old behavior where `fill_status` was implicitly true before
// protocol version 6. Can be cleaned up once version 6 is outside the
// compatibility window.
if (version_.protocol_version() < 6) {
execute_options.fill_status = true;
}

std::optional<tsl::RCReference<DeviceList>> devices;
if (!execute.device_ids().empty()) {
Expand Down

0 comments on commit a4cb02b

Please sign in to comment.