mirror of
				https://github.com/gristlabs/grist-core.git
				synced 2025-06-13 20:53:59 +00:00 
			
		
		
		
	(core) avoid typeorm's .save() method for relation with multi-column primary key
Summary:
A recently added stress test ("deletes documents reasonably quickly" in removedAt.ts) is sporadically failing under postgres.  It looks like typeorm's .save() method is in some way unreliable when setting a table with multi-column primary keys, via a ManyToMany relation. This diff replaces the .save() with explicit inserts/deletes.
I modified _repairWorkspaceGuests recently, so thought that change might have been the problem. However under the stress test, failures occur as often in _repairOrgGuests (not changed recently) as in _repairWorkspaceGuests (changed recently).
For reference, see schema diagram at https://grist.quip.com/wWpRAMe058Nl/Home-DB (the table being updated is `group_users`).
Possibly related issue: https://github.com/typeorm/typeorm/issues/4122
Test Plan:
After this change, stress test runs well on postgres locally (no failure 70 iterations); before it would fail on postgres within 3 iterations typically.
Separately: I gave a test that failed a little more time to return, and confirmed it was no slower on average, so I think it was unrelated.
Reviewers: jarek
Reviewed By: jarek
Differential Revision: https://phab.getgrist.com/D2848
			
			
This commit is contained in:
		
							parent
							
								
									29c2b35dcc
								
							
						
					
					
						commit
						2feef7f780
					
				@ -2423,8 +2423,9 @@ export class HomeDBManager extends EventEmitter {
 | 
			
		||||
      // Get guest group for workspace.
 | 
			
		||||
      const wsQuery = this._workspace(scope, wsId, {manager})
 | 
			
		||||
      .leftJoinAndSelect('workspaces.aclRules', 'acl_rules')
 | 
			
		||||
      .leftJoinAndSelect('acl_rules.group', 'groups');
 | 
			
		||||
       const workspace: Workspace = (await wsQuery.getOne())!;
 | 
			
		||||
      .leftJoinAndSelect('acl_rules.group', 'groups')
 | 
			
		||||
      .leftJoinAndSelect('groups.memberUsers', 'users');
 | 
			
		||||
      const workspace: Workspace = (await wsQuery.getOne())!;
 | 
			
		||||
      const wsGuestGroup = workspace.aclRules.map(aclRule => aclRule.group)
 | 
			
		||||
        .find(_grp => _grp.name === roles.GUEST);
 | 
			
		||||
      if (!wsGuestGroup) {
 | 
			
		||||
@ -2440,8 +2441,8 @@ export class HomeDBManager extends EventEmitter {
 | 
			
		||||
        .leftJoinAndSelect('doc_groups.memberUsers', 'doc_users')
 | 
			
		||||
        .andWhere('doc_users.id is not null');
 | 
			
		||||
      const wsWithDocs = await wsWithDocsQuery.getOne();
 | 
			
		||||
      wsGuestGroup.memberUsers = this._filterEveryone(getResourceUsers(wsWithDocs?.docs || []));
 | 
			
		||||
      await manager.save(wsGuestGroup);
 | 
			
		||||
      await this._setGroupUsers(manager, wsGuestGroup.id, wsGuestGroup.memberUsers,
 | 
			
		||||
                                this._filterEveryone(getResourceUsers(wsWithDocs?.docs || [])));
 | 
			
		||||
    });
 | 
			
		||||
  }
 | 
			
		||||
 | 
			
		||||
@ -2471,11 +2472,38 @@ export class HomeDBManager extends EventEmitter {
 | 
			
		||||
        throw new Error(`_repairOrgGuests error: found ${orgGroups.length} ${roles.GUEST} ACL group(s)`);
 | 
			
		||||
      }
 | 
			
		||||
      const orgGuestGroup = orgGroups[0]!;
 | 
			
		||||
      orgGuestGroup.memberUsers = this._filterEveryone(getResourceUsers(org.workspaces));
 | 
			
		||||
      await manager.save(orgGuestGroup);
 | 
			
		||||
      await this._setGroupUsers(manager, orgGuestGroup.id, orgGuestGroup.memberUsers,
 | 
			
		||||
                                this._filterEveryone(getResourceUsers(org.workspaces)));
 | 
			
		||||
    });
 | 
			
		||||
  }
 | 
			
		||||
 | 
			
		||||
  /**
 | 
			
		||||
   * Update the set of users in a group.  TypeORM's .save() method appears to be
 | 
			
		||||
   * unreliable for a ManyToMany relation with a table with a multi-column primary
 | 
			
		||||
   * key, so we make the update using explicit deletes and inserts.
 | 
			
		||||
   */
 | 
			
		||||
  private async _setGroupUsers(manager: EntityManager, groupId: number, usersBefore: User[],
 | 
			
		||||
                               usersAfter: User[]) {
 | 
			
		||||
    const userIdsBefore = new Set(usersBefore.map(u => u.id));
 | 
			
		||||
    const userIdsAfter = new Set(usersAfter.map(u => u.id));
 | 
			
		||||
    const toDelete = [...userIdsBefore].filter(id => !userIdsAfter.has(id));
 | 
			
		||||
    const toAdd = [...userIdsAfter].filter(id => !userIdsBefore.has(id));
 | 
			
		||||
    if (toDelete.length > 0) {
 | 
			
		||||
      await manager.createQueryBuilder()
 | 
			
		||||
        .delete()
 | 
			
		||||
        .from('group_users')
 | 
			
		||||
        .whereInIds(toDelete.map(id => ({user_id: id, group_id: groupId})))
 | 
			
		||||
        .execute();
 | 
			
		||||
    }
 | 
			
		||||
    if (toAdd.length > 0) {
 | 
			
		||||
      await manager.createQueryBuilder()
 | 
			
		||||
        .insert()
 | 
			
		||||
        .into('group_users')
 | 
			
		||||
        .values(toAdd.map(id => ({user_id: id, group_id: groupId})))
 | 
			
		||||
        .execute();
 | 
			
		||||
    }
 | 
			
		||||
  }
 | 
			
		||||
 | 
			
		||||
  /**
 | 
			
		||||
   * Don't add everyone@ as a guest, unless also sharing with anon@.
 | 
			
		||||
   * This means that material shared with everyone@ doesn't become
 | 
			
		||||
 | 
			
		||||
		Loading…
	
		Reference in New Issue
	
	Block a user