Fix set_max_message_length and add the unit-tests #42

Krout0n · 2023-02-11T01:50:11Z

Closes #36. This is WIP now so I submit as a draft. This PR is for adding the unit test for #32 features.

Krout0n · 2023-02-11T04:21:14Z

tests/test_set_grpc_max_size_in_bytes.py

+        'alice': {'address': '127.0.0.1:11010'},
+        'bob': {'address': '127.0.0.1:11011'},
+    }
+    fed.init(address='local', cluster=cluster, party=party, cross_silo_messages_max_size_in_bytes=10)


In the meantime, I try a very small value for cross_silo_messages_max_size_in_bytes.

tests/test_set_grpc_max_size_in_bytes.py

Krout0n · 2023-02-11T04:37:57Z

fed/barriers.py

@@ -123,6 +123,7 @@ async def send_data_grpc(
 ):
    tls_enabled = fed_utils.tls_enabled(tls_config)
    grpc_options = get_grpc_options(retry_policy=retry_policy)
+    logger.info("{grpc_options}")


my ex-PR(#32) is broken now??

(SendProxyActor pid=22207) INFO:fed.barriers:[('grpc.max_send_message_length', 524288000), ('grpc.max_receive_message_length', 524288000), ('grpc.enable_retries', 1), ('grpc.service_config', '{"methodConfig": [{"name": [{"service": "GrpcService"}], "retryPolicy": {"maxAttempts": 5, "initialBackoff": "5s", "maxBackoff": "30s", "backoffMultiplier": 2, "retryableStatusCodes": ["UNAVAILABLE"]}}]}')]

This reverts commit 18f76de.

Krout0n

Yes, it is broken now...

2023-02-11 05:00:57 INFO grpc_options.py:47 [alice] -- the max length is set 10 10
2023-02-11 05:00:57 INFO grpc_options.py:47 [bob] -- the max length is set 10 10
(RecverProxyActor pid=25835) INFO:root:the max send length 524288000
(RecverProxyActor pid=25835) INFO:root:the max recieve length 524288000

(rayfed) krouton@rayfed:~/rayfed$ python3 -m pytest -v -s tests/tests_set_grpc_max_size_in_bytes.py 
============================================================================================================== test session starts ===============================================================================================================
platform linux -- Python 3.8.16, pytest-7.2.1, pluggy-1.0.0 -- /home/krouton/anaconda3/envs/rayfed/bin/python3
cachedir: .pytest_cache
rootdir: /home/krouton/rayfed
collected 1 item                                                                                                                                                                                                                                 

tests/tests_set_grpc_max_size_in_bytes.py::test_grpc_max_size 2023-02-11 05:00:55,133   INFO worker.py:1518 -- Started a local Ray instance.
2023-02-11 05:00:55,136 INFO worker.py:1518 -- Started a local Ray instance.
2023-02-11 05:00:57 INFO grpc_options.py:47 [alice] --  the max length is set 10 10
2023-02-11 05:00:57 INFO grpc_options.py:47 [bob] --  the max length is set 10 10
2023-02-11 05:00:58 INFO barriers.py:309 [alice] --  RecverProxy was successfully created.
(RecverProxyActor pid=25835) INFO:root:the max send length 524288000
(RecverProxyActor pid=25835) INFO:root:the max recieve length 524288000
(RecverProxyActor pid=25835) INFO:fed.barriers:Successfully start Grpc service without credentials.
2023-02-11 05:00:58 INFO barriers.py:309 [bob] --  RecverProxy was successfully created.
(RecverProxyActor pid=25862) INFO:root:the max send length 524288000
(RecverProxyActor pid=25862) INFO:root:the max recieve length 524288000
(RecverProxyActor pid=25862) INFO:fed.barriers:Successfully start Grpc service without credentials.
2023-02-11 05:00:59 INFO barriers.py:344 [alice] --  SendProxy was successfully created.
2023-02-11 05:00:59 INFO cleanup.py:84 [alice] --  Start check sending thread.
2023-02-11 05:00:59 INFO cleanup.py:90 [alice] --  Start check sending monitor thread.
2023-02-11 05:00:59 INFO barriers.py:344 [bob] --  SendProxy was successfully created.
2023-02-11 05:00:59 INFO cleanup.py:84 [bob] --  Start check sending thread.
2023-02-11 05:01:00 INFO cleanup.py:90 [bob] --  Start check sending monitor thread.
(SendProxyActor pid=25918) INFO:root:the max send length 524288000
(SendProxyActor pid=25918) INFO:root:the max recieve length 524288000
2023-02-11 05:01:00 INFO cleanup.py:102 [bob] --  Notify check sending thread to exit.
2023-02-11 05:01:00 INFO cleanup.py:65 [bob] --  Check sending thread was exited.
2023-02-11 05:01:00 INFO cleanup.py:102 [alice] --  Notify check sending thread to exit.
2023-02-11 05:01:01 INFO cleanup.py:65 [alice] --  Check sending thread was exited.
2023-02-11 05:01:02 INFO api.py:201 [bob] --  Shutdowned ray.
2023-02-11 05:01:03 INFO api.py:201 [alice] --  Shutdowned ray.
PASSED

Krout0n · 2023-02-13T02:23:36Z

fed/_private/grpc_options.py

@@ -30,29 +31,43 @@
 _GRPC_MAX_SEND_MESSAGE_LENGTH = _DEFAULT_GRPC_MAX_SEND_MESSAGE_LENGTH
 _GRPC_MAX_RECEIVE_MESSAGE_LENGTH = _DEFAULT_GRPC_MAX_RECEIVE_MESSAGE_LENGTH

+logging.info(f"global {_GRPC_MAX_SEND_MESSAGE_LENGTH} {_GRPC_MAX_RECEIVE_MESSAGE_LENGTH}")


[memo / fixme]
Module reloading is occurred, that means it seems to be what's causing the bug, resetting the global variables.

(rayfed) krouton@rayfed:~/rayfed$ python3 -m pytest -v -s tests/tests_set_grpc_max_size_in_bytes.py ============================================================================================================== test session starts ============================================================================================================== platform linux -- Python 3.8.16, pytest-7.2.1, pluggy-1.0.0 -- /home/krouton/anaconda3/envs/rayfed/bin/python3 cachedir: .pytest_cache rootdir: /home/krouton/rayfed collected 1 item tests/tests_set_grpc_max_size_in_bytes.py::test_grpc_max_size 2023-02-13 02:17:12,000 INFO worker.py:1518 -- Started a local Ray instance. 2023-02-13 02:17:13,029 INFO worker.py:1518 -- Started a local Ray instance. 2023-02-13 02:17:14 INFO grpc_options.py:43 [alice] -- set max message length 10 2023-02-13 02:17:14 INFO grpc_options.py:50 [alice] -- the max length is set 10 10 2023-02-13 02:17:15 INFO grpc_options.py:43 [bob] -- set max message length 10 2023-02-13 02:17:15 INFO grpc_options.py:50 [bob] -- the max length is set 10 10 2023-02-13 02:17:15 INFO barriers.py:309 [alice] -- RecverProxy was successfully created. (pid=7127) INFO:root:global 524288000 524288000 (RecverProxyActor pid=7127) INFO:root:the max send length 524288000 get_grpc_max524288000 (RecverProxyActor pid=7127) INFO:root:the max recieve length 524288000 get_grpc_max524288000 (RecverProxyActor pid=7127) INFO:fed.barriers:Successfully start Grpc service without credentials. 2023-02-13 02:17:16 INFO barriers.py:309 [bob] -- RecverProxy was successfully created. (pid=7252) INFO:root:global 524288000 524288000 (RecverProxyActor pid=7252) INFO:root:the max send length 524288000 get_grpc_max524288000 (RecverProxyActor pid=7252) INFO:root:the max recieve length 524288000 get_grpc_max524288000 (RecverProxyActor pid=7252) INFO:fed.barriers:Successfully start Grpc service without credentials. 2023-02-13 02:17:16 INFO barriers.py:344 [alice] -- SendProxy was successfully created. (pid=7305) INFO:root:global 524288000 524288000 2023-02-13 02:17:16 INFO cleanup.py:84 [alice] -- Start check sending thread. 2023-02-13 02:17:16 INFO cleanup.py:90 [alice] -- Start check sending monitor thread. 2023-02-13 02:17:17 INFO barriers.py:344 [bob] -- SendProxy was successfully created. 2023-02-13 02:17:17 INFO cleanup.py:84 [bob] -- Start check sending thread. 2023-02-13 02:17:17 INFO cleanup.py:90 [bob] -- Start check sending monitor thread. (pid=7313) INFO:root:global 524288000 524288000 (SendProxyActor pid=7305) INFO:root:the max send length 524288000 get_grpc_max524288000 (SendProxyActor pid=7305) INFO:root:the max recieve length 524288000 get_grpc_max524288000 2023-02-13 02:17:17 INFO cleanup.py:102 [bob] -- Notify check sending thread to exit. 2023-02-13 02:17:17 INFO cleanup.py:102 [alice] -- Notify check sending thread to exit. 2023-02-13 02:17:17 INFO cleanup.py:65 [bob] -- Check sending thread was exited. 2023-02-13 02:17:18 INFO cleanup.py:65 [alice] -- Check sending thread was exited. 2023-02-13 02:17:20 INFO api.py:201 [bob] -- Shutdowned ray. 2023-02-13 02:17:20 INFO api.py:201 [alice] -- Shutdowned ray. PASSED ============================================================================================================== 1 passed in 11.36s =============================================================================================================== (rayfed) krouton@rayfed:~/rayfed$

jovany-wang · 2023-02-13T03:10:23Z

Hi @Krout0n ,thanks for the efforts. If this is ready for review, please let us know.

Krout0n · 2023-02-13T05:10:29Z

Hi @jovany-wang ! I apologize for my soliloquizing... I was only targeting adding unit tests in this PR, but I'll make sure to include bug fixes in the scope as well. So I will notice you when it's ready for review!

jovany-wang · 2023-02-21T13:43:09Z

replaced by #63

jovany-wang · 2023-02-21T13:44:47Z

Hi @Krout0n due to that I have no access to your repo, so I create a new PR. #63

Krout0n added 2 commits February 11, 2023 10:55

WIP:

3b5a82b

WIP: RecursionError: maximum recursion depth exceeded in comparison

11413f7

Krout0n force-pushed the add-unittest-for-set_max_message_length branch from 956a3a1 to 11413f7 Compare February 11, 2023 04:01

WIP: the test is passed, but it must be failed now

5ce10d7

Krout0n commented Feb 11, 2023

View reviewed changes

[Revert me] log grpc options

18f76de

Krout0n commented Feb 11, 2023

View reviewed changes

Krout0n added 2 commits February 11, 2023 13:58

Revert "[Revert me] log grpc options"

ca77b82

This reverts commit 18f76de.

[REVERT ME] add logging in grpc_options

6f8280b

Krout0n commented Feb 11, 2023

View reviewed changes

Krout0n added 2 commits February 11, 2023 15:04

Rename the test file

fc5490e

[REVERT ME] add logging.info when importing

1108a1e

Krout0n commented Feb 13, 2023

View reviewed changes

Krout0n changed the title ~~Add a unit-test for set_max_message_length~~ Fix, set_max_message_length and add the unit-tests Feb 13, 2023

Krout0n changed the title ~~Fix, set_max_message_length and add the unit-tests~~ Fix set_max_message_length and add the unit-tests Feb 13, 2023

Krout0n mentioned this pull request Feb 15, 2023

Bug: the cross_silo_messages_max_size_in_bytes passed in fed.init is not reflected #46

Closed

jovany-wang mentioned this pull request Feb 21, 2023

Fix cross_silo_messages_max_size_in_bytes does not work. #63

Merged

jovany-wang closed this Feb 21, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix set_max_message_length and add the unit-tests #42

Fix set_max_message_length and add the unit-tests #42

Krout0n commented Feb 11, 2023 •

edited

Loading

Krout0n Feb 11, 2023

Krout0n Feb 11, 2023

Krout0n left a comment

Krout0n Feb 13, 2023 •

edited

Loading

jovany-wang commented Feb 13, 2023

Krout0n commented Feb 13, 2023

jovany-wang commented Feb 21, 2023

jovany-wang commented Feb 21, 2023

Fix set_max_message_length and add the unit-tests #42

Fix set_max_message_length and add the unit-tests #42

Conversation

Krout0n commented Feb 11, 2023 • edited Loading

Krout0n Feb 11, 2023

Choose a reason for hiding this comment

Krout0n Feb 11, 2023

Choose a reason for hiding this comment

Krout0n left a comment

Choose a reason for hiding this comment

Krout0n Feb 13, 2023 • edited Loading

Choose a reason for hiding this comment

jovany-wang commented Feb 13, 2023

Krout0n commented Feb 13, 2023

jovany-wang commented Feb 21, 2023

jovany-wang commented Feb 21, 2023

Krout0n commented Feb 11, 2023 •

edited

Loading

Krout0n Feb 13, 2023 •

edited

Loading