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

Validate session with flint datasource passed in async job request #2448

Merged
merged 3 commits into from
Dec 6, 2023
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -81,7 +81,8 @@ public DispatchQueryResponse submit(
if (dispatchQueryRequest.getSessionId() != null) {
// get session from request
SessionId sessionId = new SessionId(dispatchQueryRequest.getSessionId());
Optional<Session> createdSession = sessionManager.getSession(sessionId);
Optional<Session> createdSession =
sessionManager.getSession(sessionId, dispatchQueryRequest.getDatasource());
if (createdSession.isPresent()) {
session = createdSession.get();
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -35,9 +35,27 @@ public Session createSession(CreateSessionRequest request) {
return session;
}

public Optional<Session> getSession(SessionId sid) {
/**
* Retrieves the session associated with the given session ID.
*
* <p>This method is particularly used in scenarios where the data source encoded in the session
* ID is deemed untrustworthy. It allows for the safe retrieval of session details based on a
* known and validated session ID, rather than relying on potentially outdated data source
* information.
*
* <p>For more context on the use case and implementation, refer to the documentation here:
* https://tinyurl.com/bdh6s834
*
* @param sid The unique identifier of the session. It is used to fetch the corresponding session
* details.
* @param dataSourceName The name of the data source. This parameter is utilized in the session
* retrieval process.
* @return An Optional containing the session associated with the provided session ID. Returns an
* empty Optional if no matching session is found.
*/
public Optional<Session> getSession(SessionId sid, String dataSourceName) {
Optional<SessionModel> model =
StateStore.getSession(stateStore, sid.getDataSourceName()).apply(sid.getSessionId());
StateStore.getSession(stateStore, dataSourceName).apply(sid.getSessionId());
if (model.isPresent()) {
InteractiveSession session =
InteractiveSession.builder()
Expand All @@ -51,6 +69,22 @@ public Optional<Session> getSession(SessionId sid) {
return Optional.empty();
}

/**
* Retrieves the session associated with the provided session ID.
*
* <p>This method is utilized specifically in scenarios where the data source information encoded
* in the session ID is considered trustworthy. It ensures the retrieval of session details based
* on the session ID, relying on the integrity of the data source information contained within it.
*
* @param sid The session ID used to identify and retrieve the corresponding session. It is
* expected to contain valid and trusted data source information.
* @return An Optional containing the session associated with the provided session ID. If no
* session is found that matches the session ID, an empty Optional is returned.
*/
public Optional<Session> getSession(SessionId sid) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

it is only for testing?

Copy link
Collaborator

Choose a reason for hiding this comment

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

if yes, add an annotation.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It is used by other src code. Not only for testing.

return getSession(sid, sid.getDataSourceName());
}

// todo, keep it only for testing, will remove it later.
public boolean isEnabled() {
return true;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -383,6 +383,44 @@ public void recreateSessionIfNotReady() {
assertNotEquals(second.getSessionId(), third.getSessionId());
}

@Test
public void submitQueryWithDifferentDataSourceSessionWillCreateNewSession() {
LocalEMRSClient emrsClient = new LocalEMRSClient();
AsyncQueryExecutorService asyncQueryExecutorService =
createAsyncQueryExecutorService(emrsClient);

// enable session
enableSession(true);

// 1. create async query.
CreateAsyncQueryResponse first =
asyncQueryExecutorService.createAsyncQuery(
new CreateAsyncQueryRequest(
"SHOW SCHEMAS IN " + DATASOURCE, DATASOURCE, LangType.SQL, null));
assertNotNull(first.getSessionId());

// set sessionState to RUNNING
setSessionState(first.getSessionId(), SessionState.RUNNING);

// 2. reuse session id
CreateAsyncQueryResponse second =
asyncQueryExecutorService.createAsyncQuery(
new CreateAsyncQueryRequest(
"SHOW SCHEMAS IN " + DATASOURCE, DATASOURCE, LangType.SQL, first.getSessionId()));

assertEquals(first.getSessionId(), second.getSessionId());

// set sessionState to RUNNING
setSessionState(second.getSessionId(), SessionState.RUNNING);

// 3. given different source, create a new session id
CreateAsyncQueryResponse third =
asyncQueryExecutorService.createAsyncQuery(
new CreateAsyncQueryRequest(
"SHOW SCHEMAS IN " + DSOTHER, DSOTHER, LangType.SQL, second.getSessionId()));
assertNotEquals(second.getSessionId(), third.getSessionId());
}

@Test
public void submitQueryInInvalidSessionWillCreateNewSession() {
LocalEMRSClient emrsClient = new LocalEMRSClient();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -314,7 +314,7 @@ void testDispatchSelectQueryReuseSession() {
doReturn(true).when(sessionManager).isEnabled();
doReturn(Optional.of(session))
.when(sessionManager)
.getSession(eq(new SessionId(MOCK_SESSION_ID)));
.getSession(eq(new SessionId(MOCK_SESSION_ID)), eq("my_glue"));
doReturn(new SessionId(MOCK_SESSION_ID)).when(session).getSessionId();
doReturn(new StatementId(MOCK_STATEMENT_ID)).when(session).submit(any());
when(session.getSessionModel().getJobId()).thenReturn(EMR_JOB_ID);
Expand Down
Loading