[services] Rust Integration - Backup - Rust - Multiple get clients
ClosedPublic
Actions

Authored by • karol on Sep 2 2022, 3:52 AM.

Details

Reviewers

• jon
tomek
• max
varun

Commits

rCOMM7143223f7246: [services] Rust Integration - Backup - Rust - Multiple get clients

Summary

Depends on D5036

In order to be able to handle multiple connections at the same time, we have to store a collection of clients in the rust state instead of having just one.

Here, I'm adding the collection to the get blob client.

Test Plan

The backup service will not build successfully because of invalid calls from the c++ but I decided to divide the code so the diffs are not too big. This is fixed in the next diff in the stack - D5038

This will work:

cd services/backup/blob_client
cargo check

Diff Detail

Repository

rCOMM Comm

Lint

Lint Not Applicable

Unit

Tests Not Applicable

Event Timeline

• karol created this revision.Sep 2 2022, 3:52 AM

Herald added a reviewer: • jon. · View Herald TranscriptSep 2 2022, 3:52 AM

Herald added subscribers: • abosh, atul, tomek, ashoat. · View Herald Transcript

Harbormaster returned this revision to the author for changes because remote builds failed.Sep 2 2022, 3:56 AM

Harbormaster failed remote builds in B11831: Diff 16236!

Backup doesn't build but this is explained in the test plan section.

tomek requested changes to this revision.Sep 5 2022, 8:09 AM

tomek added inline comments.

services/backup/blob_client/src/get_client.rs
27 ↗	(On Diff #16236)	Can `RwLock` improve the performance?
38 ↗	(On Diff #16236)	This should be fixed in a diff that introduced it
146–157 ↗	(On Diff #16236)	Are we holding a lock to the whole `CLIENTS` while we're executing blocking function?

This revision now requires changes to proceed.Sep 5 2022, 8:09 AM

• karol added inline comments.Sep 6 2022, 7:14 AM

services/backup/blob_client/src/get_client.rs
27 ↗	(On Diff #16236)	https://linear.app/comm/issue/ENG-1741/consider-using-rwlock-instead-of-mutex
38 ↗	(On Diff #16236)	I think this change should be reverted actually.
146–157 ↗	(On Diff #16236)	Apparently. We may want to re-check the performance possible issues like this.

revert incorrect change

Harbormaster failed remote builds in B11922: Diff 16368!Sep 6 2022, 7:19 AM

tomek accepted this revision.Sep 9 2022, 5:11 AM

tomek added inline comments.

services/backup/blob_client/src/get_client.rs
146–157 ↗	(On Diff #16236)	Ok, so let's do that. If we're locking here that might affect performance significantly.

This revision is now accepted and ready to land.Sep 9 2022, 5:11 AM

tomek added inline comments.Sep 13 2022, 10:53 AM

services/backup/blob_client/src/get_client.rs
146–157 ↗	(On Diff #16236)	Do we have a task for this investigation?

• karol added inline comments.Sep 14 2022, 4:03 AM

services/backup/blob_client/src/get_client.rs
146–157 ↗	(On Diff #16236)	https://linear.app/comm/issue/ENG-1811/lock-the-clients-collection-in-the-most-efficient-way

rebase

Harbormaster failed remote builds in B12158: Diff 16686!Sep 15 2022, 1:30 AM

The code looks ok, but I'm confused why this diff introduces things that are replaced later. Especially, raw pointers and unsafe functions. Is this diff going to be updated?

Also the CI fails for this diff.

services/backup/blob_client/src/get_client.rs
36 ↗	(On Diff #16686)	We're not mutating, so `&str` is more appropriate

format

Harbormaster failed remote builds in B12179: Diff 16707!Sep 15 2022, 9:16 AM

Also the CI fails for this diff.

The CI fails because we don't use the changes from rust in c++ yet, we do that in D5038 but I didn't want to make this diff too big. On the other hand, the diff philosophy is that every single diff should not break the master branch... To speed things up, I'll just land them together, sorry, maybe I made a mistake and should've done that differently.

services/backup/blob_client/src/get_client.rs
36 ↗	(On Diff #16686)	D5092

rebase

This revision was landed with ongoing or failed builds.Sep 16 2022, 1:55 AM

Closed by commit rCOMM7143223f7246: [services] Rust Integration - Backup - Rust - Multiple get clients (authored by • karol). · Explain Why

This revision was automatically updated to reflect the committed changes.

• karol added a commit: rCOMM7143223f7246: [services] Rust Integration - Backup - Rust - Multiple get clients.

Harbormaster failed remote builds in B12203: Diff 16738!Sep 16 2022, 1:57 AM

Revision Contents
Changeset List

Path

Size

services/

backup/

blob_client/

src/

get_client.rs

73 lines

lib.rs

8 lines

Diff 16744

View Options

services/backup/blob_client/src/get_client.rs

	Show All 35 Lines
	import type { CommunityCreationMembersScreenParams } from '../community-creation/community-creation-members.react.js';			import type { CommunityCreationMembersScreenParams } from '../community-creation/community-creation-members.react.js';
	import type { ManagePublicLinkScreenParams } from '../invite-links/manage-public-link-screen.react.js';			import type { ManagePublicLinkScreenParams } from '../invite-links/manage-public-link-screen.react.js';
	import type { ViewInviteLinksScreenParams } from '../invite-links/view-invite-links-screen.react.js';			import type { ViewInviteLinksScreenParams } from '../invite-links/view-invite-links-screen.react.js';
	import type { ChatCameraModalParams } from '../media/chat-camera-modal.react.js';			import type { ChatCameraModalParams } from '../media/chat-camera-modal.react.js';
	import type { ImageModalParams } from '../media/image-modal.react.js';			import type { ImageModalParams } from '../media/image-modal.react.js';
	import type { ThreadAvatarCameraModalParams } from '../media/thread-avatar-camera-modal.react.js';			import type { ThreadAvatarCameraModalParams } from '../media/thread-avatar-camera-modal.react.js';
	import type { VideoPlaybackModalParams } from '../media/video-playback-modal.react.js';			import type { VideoPlaybackModalParams } from '../media/video-playback-modal.react.js';
	import type { CustomServerModalParams } from '../profile/custom-server-modal.react.js';			import type { CustomServerModalParams } from '../profile/custom-server-modal.react.js';
				import type { KeyserverSelectionBottomSheetParams } from '../profile/keyserver-selection-bottom-sheet.react.js';
	import type { UserRelationshipTooltipModalParams } from '../profile/user-relationship-tooltip-modal.react.js';			import type { UserRelationshipTooltipModalParams } from '../profile/user-relationship-tooltip-modal.react.js';
	import type { ChangeRolesScreenParams } from '../roles/change-roles-screen.react.js';			import type { ChangeRolesScreenParams } from '../roles/change-roles-screen.react.js';
	import type { CommunityRolesScreenParams } from '../roles/community-roles-screen.react.js';			import type { CommunityRolesScreenParams } from '../roles/community-roles-screen.react.js';
	import type { CreateRolesScreenParams } from '../roles/create-roles-screen.react.js';			import type { CreateRolesScreenParams } from '../roles/create-roles-screen.react.js';
	import type { MessageSearchParams } from '../search/message-search.react.js';			import type { MessageSearchParams } from '../search/message-search.react.js';
	import type { UserProfileAvatarModalParams } from '../user-profile/user-profile-avatar-modal.react.js';			import type { UserProfileAvatarModalParams } from '../user-profile/user-profile-avatar-modal.react.js';
	import type { UserProfileBottomSheetParams } from '../user-profile/user-profile-bottom-sheet.react.js';			import type { UserProfileBottomSheetParams } from '../user-profile/user-profile-bottom-sheet.react.js';

	▲ Show 20 Lines • Show All 85 Lines • ▼ Show 20 Lines
	export const QRCodeSignInNavigatorRouteName = 'QRCodeSignInNavigator';			export const QRCodeSignInNavigatorRouteName = 'QRCodeSignInNavigator';
	export const QRCodeScreenRouteName = 'QRCodeScreen';			export const QRCodeScreenRouteName = 'QRCodeScreen';
	export const UserProfileBottomSheetNavigatorRouteName =			export const UserProfileBottomSheetNavigatorRouteName =
	'UserProfileBottomSheetNavigator';			'UserProfileBottomSheetNavigator';
	export const UserProfileBottomSheetRouteName = 'UserProfileBottomSheet';			export const UserProfileBottomSheetRouteName = 'UserProfileBottomSheet';
	export const UserProfileAvatarModalRouteName = 'UserProfileAvatarModal';			export const UserProfileAvatarModalRouteName = 'UserProfileAvatarModal';
	export const KeyserverSelectionListRouteName = 'KeyserverSelectionList';			export const KeyserverSelectionListRouteName = 'KeyserverSelectionList';
	export const AddKeyserverRouteName = 'AddKeyserver';			export const AddKeyserverRouteName = 'AddKeyserver';
				export const KeyserverSelectionBottomSheetRouteName =
				'KeyserverSelectionBottomSheet';

	export type RootParamList = {			export type RootParamList = {
	+LoggedOutModal: void,			+LoggedOutModal: void,
	+App: void,			+App: void,
	+ThreadPickerModal: ThreadPickerModalParams,			+ThreadPickerModal: ThreadPickerModalParams,
	+AddUsersModal: AddUsersModalParams,			+AddUsersModal: AddUsersModalParams,
	+CustomServerModal: CustomServerModalParams,			+CustomServerModal: CustomServerModalParams,
	+ColorSelectorModal: ColorSelectorModalParams,			+ColorSelectorModal: ColorSelectorModalParams,
	+ComposeSubchannelModal: ComposeSubchannelModalParams,			+ComposeSubchannelModal: ComposeSubchannelModalParams,
	+SidebarListModal: SidebarListModalParams,			+SidebarListModal: SidebarListModalParams,
	+ImagePasteModal: ImagePasteModalParams,			+ImagePasteModal: ImagePasteModalParams,
	+TermsAndPrivacyModal: TermsAndPrivacyModalParams,			+TermsAndPrivacyModal: TermsAndPrivacyModalParams,
	+SubchannelsListModal: SubchannelListModalParams,			+SubchannelsListModal: SubchannelListModalParams,
	+MessageReactionsModal: MessageReactionsModalParams,			+MessageReactionsModal: MessageReactionsModalParams,
	+Registration: void,			+Registration: void,
	+CommunityCreation: void,			+CommunityCreation: void,
	+InviteLinkModal: InviteLinkModalParams,			+InviteLinkModal: InviteLinkModalParams,
	+InviteLinkNavigator: void,			+InviteLinkNavigator: void,
	+RolesNavigator: void,			+RolesNavigator: void,
	+QRCodeSignInNavigator: void,			+QRCodeSignInNavigator: void,
	+UserProfileBottomSheetNavigator: void,			+UserProfileBottomSheetNavigator: void,
	+TunnelbrokerMenu: void,			+TunnelbrokerMenu: void,
				+KeyserverSelectionBottomSheet: KeyserverSelectionBottomSheetParams,
	};			};

	export type MessageTooltipRouteNames =			export type MessageTooltipRouteNames =
	\| typeof RobotextMessageTooltipModalRouteName			\| typeof RobotextMessageTooltipModalRouteName
	\| typeof MultimediaMessageTooltipModalRouteName			\| typeof MultimediaMessageTooltipModalRouteName
	\| typeof TextMessageTooltipModalRouteName;			\| typeof TextMessageTooltipModalRouteName;

	export const PinnableMessageTooltipRouteNames = [			export const PinnableMessageTooltipRouteNames = [
	▲ Show 20 Lines • Show All 161 Lines • Show Last 20 Lines