Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(ConnectSession): add method nullSafeGetSkipPermissionCheck, add param maxConcurrentTaskCount in createPhysicalConnectionSession #3897

Open
wants to merge 1 commit into
base: dev/obcloud_24v5
Choose a base branch
from

Conversation

CHLK
Copy link
Collaborator

@CHLK CHLK commented Nov 22, 2024

…dd param maxConcurrentTaskCount in createPhysicalConnectionSession

What type of PR is this?

type-feature

What this PR does / why we need it:

support execute chart

  • add method nullSafeGetSkipPermissionCheck;
  • add param maxConcurrentTaskCount in createPhysicalConnectionSession

Which issue(s) this PR fixes:

Fixes #

Special notes for your reviewer:

Additional documentation e.g., usage docs, etc.:


…dd param maxConcurrentTaskCount in createPhysicalConnectionSession
@CHLK CHLK self-assigned this Nov 22, 2024
@CHLK CHLK added the type-feature The functionality to be implemented label Nov 22, 2024
@CHLK CHLK modified the milestones: obcloud-202408, obcloud_202410 Nov 22, 2024
if (createConnectionSessionSupplier == null) {
throw new NotFoundException(ResourceType.ODC_SESSION, "ID", sessionId);
}
Lock lock = this.sessionId2Lock.computeIfAbsent(sessionId, s -> new ReentrantLock());
Copy link
Collaborator

@LioRoger LioRoger Nov 29, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this lock logic can't protect session acquire in same behavior

  1. if first attempt get session failed, code goto line 408, but session is created ,will this session should be cancelExpire? if not so, this may cause session used by multi user
  2. each session id will create a lock in sessionId2Lock, this will cause memory leak, is that expected?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type-feature The functionality to be implemented
Projects
Status: New
Development

Successfully merging this pull request may close these issues.

2 participants