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

fix(llm): enable tasks concurrency configs in Gradio #188

Open
wants to merge 11 commits into
base: main
Choose a base branch
from

Conversation

Kryst4lDem0ni4s
Copy link
Contributor

@Kryst4lDem0ni4s Kryst4lDem0ni4s commented Mar 2, 2025

fix #176

Cause of the Problem:
Blocking and Queuing Behavior: Gradio, by default, has a concurrency limit of 1. This causes blocking when multiple users or windows are accessing the application simultaneously.
No Asynchronous Execution: The current implementation does not specify concurrency_limit, leading to sequential execution.
Queue Configuration: If queue() is not configured properly, tasks are processed one at a time.

Changed:

log_path = "logs/llm-server.log"
    try:
        with open(log_path, "r", encoding='utf-8') as f:
            return ''.join(deque(f, maxlen=lines))
    except FileNotFoundError:
        log.critical("Log file not found: %s", log_path)
        return "LLM Server log file not found."

to

log_path = "logs/llm-server.log"
    try:
        with open(log_path, "r", encoding='utf-8') as f:
            data = f.read()
            return ''.join(deque(data, maxlen=lines))
    except FileNotFoundError:
        print(f"Log file not found: {log_path}")
        return "LLM Server log file not found."
    except UnicodeDecodeError as e:
        print(f"Decoding error occurred: {e}")
        # ignore the bad bytes and continue processing
        with open(log_path, "r", encoding='utf-8', errors='ignore') as f:
            data = f.read()
            return ''.join(deque(data, maxlen=lines))

in order to handle frequent unicode encoding errors during attempts to read logs.

Also added task ID generation for RAG functions, and enabled concurrency using Gradio Blocks in rag_block.py in rag_demo by creating a task ID generation decorator and used Gradio Blocks for concurrency control. Added logs for clear understanding. Issue: #176 #176

Closed previous PRs due to merge conflicts. please check this @imbajin

@dosubot dosubot bot added the size:L This PR changes 100-499 lines, ignoring generated files. label Mar 2, 2025
@dosubot dosubot bot added the enhancement New feature or request label Mar 2, 2025
@imbajin imbajin changed the title Enabled concurrency with Gradio with Task_ID generations for Task #176 fix(llm): enable tasks concurrency configs in Gradio Mar 2, 2025
@imbajin
Copy link
Member

imbajin commented Mar 2, 2025

we could merge after #187 done (And some tips for the individual PRs)

  1. checkout a new branch from master/main each time when u want to submit a separate PR(rather than use the previous dev branch)
  2. use fix #xxxx or close #xxx in github to link the issue & PR (u can refer the usage/synax in github's doc)
image

@Kryst4lDem0ni4s
Copy link
Contributor Author

@imbajin thanks for the tips, I was having trouble getting used to that initially

@imbajin
Copy link
Member

imbajin commented Mar 3, 2025

Also note check/fix the (CI) lint error to keep our code better

refer https://github.com/apache/incubator-hugegraph-ai#contributing

image

Comment on lines 211 to 212
# queue=True, # Enable queueing for this event
# concurrency_limit=5, # Maximum of 5 concurrent executions
Copy link
Member

@imbajin imbajin Mar 3, 2025

Choose a reason for hiding this comment

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

maybe the two lines 211~212 modification is enough for now

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Alright I'll modify it to incorporate the buttons for now.

@dosubot dosubot bot added size:M This PR changes 30-99 lines, ignoring generated files. and removed size:L This PR changes 100-499 lines, ignoring generated files. labels Mar 3, 2025
Comment on lines 94 to 103
# A decorator to wrap functions with a task id generation and logging
def with_task_id(func):
def wrapper(*args, **kwargs):
import uuid
task_id = str(uuid.uuid4())
log.info(f"New task created with id: {task_id}")
# Optionally, you could also pass the task_id to the function if needed:
# kwargs['task_id'] = task_id
return func(*args, **kwargs)
return wrapper
Copy link
Member

Choose a reason for hiding this comment

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

Is this decorator still useful now or in the future? Is the main purpose of logging newly created tasks?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes this one is useful, it's for generating concurrency ids, while the other is a more static general id useful for logging references (the one in the button) and can be modified or removed.

Copy link
Member

Choose a reason for hiding this comment

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

Yes this one is useful, it's for generating concurrency ids, while the other is a more static general id useful for logging references (the one in the button) and can be modified or removed.

We can keep it, but I still don't understand the purpose of recording this taskID separately in the click function? It seems that there is no significant difference even if I don't use this decorator and record it?

BTW, our logs are already too numerous and need to be streamlined as much as possible 😄

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Understood, I'll try to streamline the logs next.
And you're right, there isnt any significant usage of the click function's taskID, it is merely for identifying the ongoing process so that it is clearer to the dev, which process' ID was being generated, but we can reduce this too if it is unneccesary. As for the decorator, the task_id = str(uuid.uuid4()) is being used to identify any concurrent processes being run, so that we can differentiate ongoing processes per task_id for further debugging as per #176 . I hope this is useful for further development and scaling

Copy link
Member

Choose a reason for hiding this comment

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

Understood, I'll try to streamline the logs next.
And you're right, there isnt any significant usage of the click function's taskID, it is merely for identifying the ongoing process so that it is clearer to the dev, which process' ID was being generated, but we can reduce this too if it is unneccesary. As for the decorator, the task_id = str(uuid.uuid4()) is being used to identify any concurrent processes being run, so that we can differentiate ongoing processes per task_id for further debugging as per #176 . I hope this is useful for further development and scaling

I haven't found out how it works in my local tests. Can you give me an example of how to use it?

@@ -200,6 +208,9 @@ def toggle_slider(enable):
example_num,
],
outputs=[raw_out, vector_only_out, graph_only_out, graph_vector_out],
queue=True, # Enable queueing for this event
concurrency_limit=5, # Maximum of 5 concurrent executions
concurrency_id="rag_answer_task"
Copy link
Member

Choose a reason for hiding this comment

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

so as here

def wrapper(*args: Any, **kwargs: Any) -> Any:
import uuid
task_id = str(uuid.uuid4())
log.info("New task created with id: %s", task_id)
Copy link
Member

Choose a reason for hiding this comment

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

I tested it locally and didn't see when this log will be output? There was no occurrence during concurrent use

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request llm python-client size:M This PR changes 30-99 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Make Gradio UI button.click run in parallel
2 participants