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

Pass query context down to region server service in Datanode #3752

Closed
MichaelScofield opened this issue Apr 19, 2024 · 8 comments · Fixed by #3829
Closed

Pass query context down to region server service in Datanode #3752

MichaelScofield opened this issue Apr 19, 2024 · 8 comments · Fixed by #3829
Assignees
Labels
good first issue Good for newcomers

Comments

@MichaelScofield
Copy link
Collaborator

What type of enhancement is this?

Refactor

What does the enhancement do?

Now the region server service in Datanode needs a meaningful query context. For example, it needs the time zone info to correctly construct datetime related udf. You can see this in RegionServerInner::handle_read. Currently we only pass down the time zone info through RegionRequestHeader, it's better to pass down the whole query context.

Implementation challenges

No response

@MichaelScofield MichaelScofield added the good first issue Good for newcomers label Apr 19, 2024
@Kelvinyu1117
Copy link
Contributor

Kelvinyu1117 commented Apr 21, 2024

I would like to try. Can you give me more details?

@WenyXu
Copy link
Member

WenyXu commented Apr 21, 2024

I would like to try. Can you give me more details?

Hi @Kelvinyu1117, I recommend reading the code for splitting and inserting values into different datanodes. It's the opposite operation of inserting, and it could be a little tricky.

@killme2008
Copy link
Contributor

I would like to try. Can you give me more details?

Hi @Kelvinyu1117, I recommend reading the code for splitting and inserting values into different datanodes. It's the opposite operation of inserting, and it could be a little tricky.

Looks like you misunderstand this issue.

@WenyXu
Copy link
Member

WenyXu commented Apr 22, 2024

Looks like you misunderstand this issue.

Oh, I see. I took it as another issue pruning partition during predicate pushdown.🥲

@Kelvinyu1117
Copy link
Contributor

Kelvinyu1117 commented Apr 22, 2024

Thanks, anyway, I am trying to understand how the Datanode interact with the region server now.

@killme2008
Copy link
Contributor

Thanks, anyway, I am trying to understand how the Datanode interact with the region server now.

Feel free to ask if you have any questions. @MichaelScofield Could you provide some related code link that may help @Kelvinyu1117 ?

@MichaelScofield
Copy link
Collaborator Author

Thanks, anyway, I am trying to understand how the Datanode interact with the region server now.

Feel free to ask if you have any questions. @MichaelScofield Could you provide some related code link that may help @Kelvinyu1117 ?

Sure.

@Kelvinyu1117 :

  • First you can define how the query context is represented in protobuf message here. And place it in message RegionRequestHeader
  • Then in MergeScanExec::to_stream, serialize the QueryContext to its protobuf counterparts, set the corresponding field in RegionRequestHeader. This is how the query context get passed down to region server(datanode).
  • Finally in RegionServerInner::handle_read, reconstruct the QueryContext from the protobuf bytes.

@Kelvinyu1117
Copy link
Contributor

Thanks, anyway, I am trying to understand how the Datanode interact with the region server now.

Feel free to ask if you have any questions. @MichaelScofield Could you provide some related code link that may help @Kelvinyu1117 ?

Sure.

@Kelvinyu1117 :

  • First you can define how the query context is represented in protobuf message here. And place it in message RegionRequestHeader
  • Then in MergeScanExec::to_stream, serialize the QueryContext to its protobuf counterparts, set the corresponding field in RegionRequestHeader. This is how the query context get passed down to region server(datanode).
  • Finally in RegionServerInner::handle_read, reconstruct the QueryContext from the protobuf bytes.

Thanks, let me try it and open an PR.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
good first issue Good for newcomers
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants