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

[Feature] Refactor Python agent UUID generation #10849

Open
5 of 6 tasks
Superskyyy opened this issue May 23, 2023 · 9 comments
Open
5 of 6 tasks

[Feature] Refactor Python agent UUID generation #10849

Superskyyy opened this issue May 23, 2023 · 9 comments
Assignees
Labels
feature New feature python Python agent related

Comments

@Superskyyy
Copy link
Member

Superskyyy commented May 23, 2023

Search before asking

  • I had searched in the issues and found no similar feature requirement.

Description

Refactoring the uuid generator shows a 30% thoroughput increase in small transactions. UUID in Python agent previously was implemented as a ID class that creates a new UUID1 instance every single time when required, a huge number of uuids is added up as a costly bottleneck that consume large portion of CPU time. New method will replicate the Java agent logic and use a global PROCESSID (uuid4) + ThreadID + Timestamp + (1-9999) as the ids plus other python-specific optimizations.

  • - UUID generation change
  • - Consider forking behavior that is different from Java applications
  • - Optimize other parts in the tracing core.

Use case

Reduce performance cost of introducing Python agent.

This probably also leads to a new release along with several smaller enhancements.

Related issues

No response

Are you willing to submit a PR?

  • Yes I am willing to submit a PR!

Code of Conduct

@Superskyyy Superskyyy added feature New feature python Python agent related labels May 23, 2023
@Superskyyy Superskyyy added this to the Python - 1.1.0 milestone May 23, 2023
@Superskyyy Superskyyy self-assigned this May 23, 2023
@wu-sheng
Copy link
Member

Timstamp + rolling increasing number may face conflict in high concurrency cases. Be careful of that. Java agent includes thread ID to avoid that.

@Superskyyy
Copy link
Member Author

Superskyyy commented May 23, 2023

Timstamp + rolling increasing number may face conflict in high concurrency cases. Be careful of that. Java agent includes thread ID to avoid that.

Thanks for reminding! I forgot to mention the threadID, it's also included in the new implementation to sync with Java agent + forking mechanism that is rather frequent in Python.

It will be in the form of f'{GlobalIdGenerator.PROCESS_ID}.{threading.get_ident()}.{GlobalIdGenerator.THREAD_ID_SEQUENCE.context.next_seq()}' - same as Java side.

@wu-sheng
Copy link
Member

Good to see. I am aware of UUID cost in Java. Good to know the same in Python.
@mrproliu I think it is worth to check on Go agent as well.

@Superskyyy
Copy link
Member Author

Good to see. I am aware of UUID cost in Java. Good to know the same in Python. @mrproliu I think it is worth to check on Go agent as well.

This issue applies to most agents, including skywalking-go (I think)

@mrproliu
Copy link
Contributor

Good to see. I am aware of UUID cost in Java. Good to know the same in Python. @mrproliu I think it is worth to check on Go agent as well.

This issue applies to most agents, including skywalking-go (I think)

Sure, it looks like a simple version of the snowflake id generator, Am I right? I need to do some performance testing in skywalking-go.

@wu-sheng
Copy link
Member

Yes, it was inspired by that but without coordinator to assign the range. Instead, we tradeoff ID length for a better performance.

@Superskyyy
Copy link
Member Author

Done on Go and Python side, do other agents need an update? @wu-sheng should i keep this issue open as a reminder.

@wu-sheng
Copy link
Member

It is fine. If people care, they would follow up. Feel free to close once your side is done.

@Superskyyy
Copy link
Member Author

It is fine. If people care, they would follow up. Feel free to close once your side is done.

Okay!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature New feature python Python agent related
Projects
None yet
Development

No branches or pull requests

4 participants