BatchDataLoader within nested resolver does not batch requests from multiple nodes correctly

[henrikx]

Is there an existing issue for this?

• [X] I have searched the existing issues

Product

Green Donut

Describe the bug

I am not sure if it is an issue with my implementation or whether there is a bug.

Given the following query:

{
  rooms(searchCriteria: {campusCode: "V", roomTypeId: 1}, first: 15)
  {
    pageInfo {
      startCursor,
      endCursor,
      hasNextPage,
      hasPreviousPage
    }
    nodes
    {
      roomCode,
      bookings
      {
        startTime,
        endTime,
        source
      }
    }
  }
}

In the Query-class I have the following resolver to get the rooms:

public class Query
{
    [UsePaging]
    public async Task<IEnumerable<api.SchemaTypes.Room>> GetRooms(RoomsByArgsDataLoader RoomsDataLoader,
        [Service] api.SchemaTypes.Room.ReservationDescriptionsConfiguration ReservationDescriptionsConfiguration,
        List<RoomsDataLoaderFetchingArgs> searchCriteria
        )
    {
        IReadOnlyList<api.DataLoaders.Roomsdb.Room[]> allRoomResults = await RoomsDataLoader.LoadAsync(searchCriteria);
        List<api.SchemaTypes.Room> results = new List<api.SchemaTypes.Room>();
        foreach (api.DataLoaders.Roomsdb.Room[] roomResults in allRoomResults)
        {
            foreach (api.DataLoaders.Roomsdb.Room room in roomResults)
            {
                results.Add(new api.SchemaTypes.Room(room, ReservationDescriptionsConfiguration));
            }
        }
        return results;
    }

This is the Room-class with the Bookings-resolver to fetch the bookings:

public class ReservationDescriptionsConfiguration
{
    public required List<int> ReservationObjectRelationTypeIds { get; set; } = new List<int>();
    public required List<int> DescriptiondefiningObjectRelationFieldIds { get; set; } = new List<int>();
    public required int RoomNameFieldId { get; set; }
}

private readonly string _roomcode;
private readonly ReservationDescriptionsConfiguration _ReservationDescriptionsConfiguration;

public Room(api.DataLoaders.Roomsdb.Room room, ReservationDescriptionsConfiguration ReservationDescriptionsConfiguration)
{
    _roomcode = room.RoomCode;
    _roomsdbroom = room;
    _ReservationDescriptionsConfiguration = ReservationDescriptionsConfiguration;
}
private DataLoaders.Roomsdb.Room _roomsdbroom;
public int? Id() => _roomsdbroom.Id;
public string RoomCode() => _roomsdbroom.RoomCode ?? _roomcode;
public string? RoomName() => _roomsdbroom.RoomName;
public int? LocationId() => _roomsdbroom.LocationId;
public int? RoomTypeId() => _roomsdbroom.RoomTypeId;
public string? CampusAbbreviation() => _roomsdbroom.Location?.Abbreviation;
public string? Campus() => _roomsdbroom.Location?.Name;

public async Task<List<UserCalendarEvent>> Bookings(DataLoaders.AzureAd.UsersByDisplayNameDataLoader aadusersbydisplaynamedataloader, DataLoaders.AzureAd.UserCalendarEventsDataLoader aadusercalendareventsdataloader, DataLoaders.bookingsystem.bookingsystemRoomByCampusRoomCodeDataLoader bookingsystemRoomByCampusRoomCodeDataLoader, DataLoaders.bookingsystem.bookingsystemReservationsByObjectDataLoader bookingsystemReservationsByObjectDataLoader, bookingsystemObjectsByIdsDataLoader bookingsystemObjectByIdsDataLoader)
{
    List<UserCalendarEvent> bookings = new List<UserCalendarEvent>();
    IReadOnlyList<Microsoft.Graph.Models.User> roomaadusers = await _aadusersbydisplaynamedataloader.LoadAsync($"{((RoomTypeId()) == 7 ? "Meetingroom" : "Resource")} {CampusAbbreviation()} {RoomCode()}");
    List<DataLoaders.bookingsystem.InternalTypes.ObjectDto> bookingsystemrooms = await _bookingsystemRoomByCampusRoomCodeDataLoader.LoadAsync($"{CampusAbbreviation()}_{RoomCode()}");
    List<DataLoaders.bookingsystem.InternalTypes.ReservationDto> reservationDtos = bookingsystemrooms.Any() ? await _bookingsystemReservationsByObjectDataLoader.LoadAsync(new Tuple<int, int>(bookingsystemrooms.First().types[0], bookingsystemrooms.First().id)) : new();
    IReadOnlyList<DataLoaders.bookingsystem.InternalTypes.ObjectDto> objectRelations = reservationDtos.Any() ? await _bookingsystemObjectByIdsDataLoader.LoadAsync(reservationDtos.Select(x => x.objects.Select(y => y.objectId)).SelectMany(x => x).Distinct().ToList()) : new List<DataLoaders.bookingsystem.InternalTypes.ObjectDto>();
    foreach (Microsoft.Graph.Models.User userresult in roomaadusers)
    {
        bookings.AddRange((await _aadusercalendareventsdataloader.LoadAsync(userresult.UserPrincipalName!)).Select(x => new UserCalendarEvent() { Source = $"Exchange ({userresult.UserPrincipalName})", StartTime = DateTime.Parse(x.Start.DateTime).ToLocalTime(), EndTime = DateTime.Parse(x.End.DateTime).ToLocalTime(), Description = x.Subject }));

    }
    foreach (DataLoaders.bookingsystem.InternalTypes.ObjectDto bookingsystemroomresult in bookingsystemrooms )
    {
        bookings.AddRange
        (
            reservationDtos.Select(x =>
                new UserCalendarEvent()
                {
                    Description = objectRelations.FirstOrDefault(y => x.objects.FirstOrDefault(z => _ReservationDescriptionsConfiguration.ReservationObjectRelationTypeIds.Contains(z.typeId))?.objectId == y.id)?
                    .fields.FirstOrDefault(y => _ReservationDescriptionsConfiguration.DescriptiondefiningObjectRelationFieldIds.Contains(y.fieldId))?.values.FirstOrDefault(),
                    Source = $"bookingsystem ({bookingsystemroomresult.fields.FirstOrDefault(y => y.fieldId == _bookingsystemReservationDescriptionsConfiguration.RoomNameFieldId).values.First()}({bookingsystemroomresult.id})) (Reservation-Id:{x.id})",
                    StartTime = DateTimeOffset.FromUnixTimeSeconds(x.begin).ToLocalTime().DateTime, EndTime = DateTimeOffset.FromUnixTimeSeconds(x.end).ToLocalTime().DateTime 
                }
            )
        );
    }
    return bookings.OrderBy(x => x.StartTime).ToList();
}

All the dataloaders in Bookings are CacheDataLoaders except for bookingsystemObjectByIdsDataLoader which is a BatchDataLoader. This is where the error occurs; When HotChocolate executes the Bookings-resolver in parallel for each node in rooms, the bookingsystemObjectByIdsDataLoader-dataloader only returns the response for some of the nodes and it seems completely random which nodes get results from the DataLoader. Additionally, batching does not work and LoadBatchAsync gets called for every node. Isn't it meant to wait until all the nodes have made their requests and then run LoadBatchAsync once for the whole query, or have i misunderstood something?

I was able to workaround this issue by making the Bookings-method synchronus, but this is not an acceptable solution because it makes batching impossible and thus the query becomes slow.

Steps to reproduce

1. Resolve a list of objects whose class contain an async nested resolver
2. Within the nested resolver use results from a dataloader as arguments to a batchdataloader

Relevant log output

No response

Additional Context?

No response

Version

13.3.0 (however this also happened in 12.16)

[michaelstaib]

I do not think that there is a bug but its due to how its used in your code. Its difficult to tell. If you think there is an issue create a very small repro that shows your issue in a simple an understandable way and we will have a look at it.

Closing this issue for now.

[henrikx]

@michaelstaib Just wanted to bring you an update to tell you that you were absolutely right and that there was no bug. Turns out I had forgotten to run HttpResponseMessage.EnsureSuccessStatusCode in my DataLoader and I was being rate-limited by the API when running asynchronously. Probably as requests were being sent almost at the same time. Can't believe I missed something so obvious. Anyways, thanks for pointing me in the right direction :)