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

feat: Remote interface interoperate distributed computing resources #143

Merged
merged 2 commits into from
May 31, 2019

Conversation

ajblane
Copy link
Collaborator

@ajblane ajblane commented Apr 25, 2019

  • make BUILD_REMOTE=1 to build libdcurl.so and remote-worker
  • make BUILD_REMOTE=1 check with RabbitMQ broker and remote-worker
  • RPC with exclusive callback queues with TTL property
  • AMQP connection management for multiple threads
  • Implement local fallback PoW when remote interface fails

Resolved #137

@ajblane ajblane requested review from marktwtn and jserv April 25, 2019 13:27
@wusyong
Copy link

wusyong commented Apr 25, 2019

Commit should be atomic. Please consider rebase to several ones. Also some test cases would be nice.

src/remote_worker.c Outdated Show resolved Hide resolved
src/remote_worker.c Outdated Show resolved Hide resolved
src/remote_worker.c Outdated Show resolved Hide resolved
src/remote_interface.c Outdated Show resolved Hide resolved
Makefile Outdated Show resolved Hide resolved
Makefile Outdated Show resolved Hide resolved
src/dcurl.c Outdated Show resolved Hide resolved
@jserv
Copy link
Member

jserv commented Apr 26, 2019

The prefix RemoteInterface is a bit long. We can safely rename it to Remote since it does not confuse.

src/remote_interface.c Outdated Show resolved Hide resolved
Makefile Outdated Show resolved Hide resolved
Makefile Outdated Show resolved Hide resolved
mk/common.mk Outdated Show resolved Hide resolved
mk/defs.mk Outdated Show resolved Hide resolved
@ajblane ajblane added this to the Water Lily milestone May 1, 2019
Makefile Show resolved Hide resolved
src/remote_worker.c Outdated Show resolved Hide resolved
@ajblane ajblane force-pushed the remote-interface branch from 84b058c to b93a855 Compare May 2, 2019 13:22
Makefile Outdated Show resolved Hide resolved
Makefile Outdated Show resolved Hide resolved
Makefile Outdated Show resolved Hide resolved
mk/remote.mk Outdated Show resolved Hide resolved
src/dcurl.c Outdated Show resolved Hide resolved
@marktwtn
Copy link
Collaborator

marktwtn commented May 3, 2019

The prefix RemoteInterface is a bit long. We can safely rename it to Remote since it does not confuse.

I saw that some functions have the name with RemoteInterface and the other have the prefix RI.
I think we should make it consistent and I prefer using Remote since it is more elegant and does not have to guess the meaning of the abbreviation RI.

src/remote_common.c Outdated Show resolved Hide resolved
@jserv jserv requested a review from wusyong May 5, 2019 18:32
@jserv
Copy link
Member

jserv commented May 5, 2019

Check list for merging this pull request:

  1. Validate the remote interface implementation by deploying in NCKU physically.
  2. Final approval by @marktwtn for design consistency.

src/remote_worker.c Outdated Show resolved Hide resolved
src/remote_interface.h Outdated Show resolved Hide resolved
@ajblane ajblane force-pushed the remote-interface branch from 5f94bc5 to 333196f Compare May 6, 2019 03:04
mk/submodule.mk Show resolved Hide resolved
@ajblane ajblane force-pushed the remote-interface branch 9 times, most recently from 7eac396 to e07bfb1 Compare May 28, 2019 09:16
Makefile Outdated Show resolved Hide resolved
@ajblane ajblane force-pushed the remote-interface branch from e07bfb1 to fa1306a Compare May 30, 2019 02:28
Makefile Outdated Show resolved Hide resolved
Makefile Outdated Show resolved Hide resolved
Makefile Outdated Show resolved Hide resolved
mk/remote.mk Outdated Show resolved Hide resolved
mk/remote.mk Outdated Show resolved Hide resolved
mkdir $(LIBRABBITMQ_PATH)/build
ifeq ($(UNAME_S),darwin)
# macOS
cd $(LIBRABBITMQ_PATH)/build && \
Copy link
Collaborator

@marktwtn marktwtn May 30, 2019

Choose a reason for hiding this comment

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

I wonder whether this is really necessary to enter the $(LIBRABBITMQ_PATH)/build directory.
It seems using $(LIBRABBITMQ_PATH) is fine.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I think that using cmake keeps simple as entering $(LIBRABBITMQ_PATH)/build directory to build and configure RabbitMQ.

Copy link
Collaborator

Choose a reason for hiding this comment

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

For example, can't it be like:

cmake -DOPENSSL_ROOT_DIR=/usr/local/opt/openssl/ -DCMAKE_INSTALL_PREFIX=./build . && \
cmake --build ./build --target install

Copy link
Collaborator Author

@ajblane ajblane May 31, 2019

Choose a reason for hiding this comment

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

An error occurs as entering $(LIBRABBITMQ_PATH) and using the above command to build as follows:

-- Found POPT: /usr/include
-- Found XMLTO: /usr/bin/xmlto
-- Could NOT find Doxygen (missing: DOXYGEN_EXECUTABLE)
-- Building rabbitmq as a shared library - yes
-- Building rabbitmq as a static library - yes
-- Configuring done
-- Generating done
-- Build files have been written to: /home/xxxx/Desktop/dcurl/deps/rabbitmq-c
Error: could not load cache

docs/remote-interface.md Outdated Show resolved Hide resolved
src/remote_worker.c Outdated Show resolved Hide resolved
src/remote_worker.c Outdated Show resolved Hide resolved
src/remote_common.c Outdated Show resolved Hide resolved
src/remote_common.c Outdated Show resolved Hide resolved
if (frame.payload.method.id != AMQP_BASIC_DELIVER_METHOD)
continue;

#if defined(ENABLE_DEBUG)
Copy link
Collaborator

@marktwtn marktwtn May 30, 2019

Choose a reason for hiding this comment

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

The ENABLE_DEBUG can be removed since you have already used the ddprintf().
At least ddprintf() should not be placed between the #if defined(ENABLE_DEBUG) ... #endif macro.

#if defined(ENABLE_DEBUG)
p = (amqp_basic_properties_t *) frame.payload.properties.decoded;
if (p->_flags & AMQP_BASIC_CONTENT_TYPE_FLAG) {
ddprintf(MSG_PREFIX "Content-type: %.*s\n", (int) p->content_type.len,
Copy link
Collaborator

Choose a reason for hiding this comment

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

Ditto.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

p is defined and used by ddprintf when make BUILD_DEBUG. Therefore, the ENABLE_DEBUG needs to be used.

@ajblane ajblane force-pushed the remote-interface branch from ff7240d to a084095 Compare May 31, 2019 01:01
ajblane added 2 commits May 30, 2019 19:06
* make BUILD_REMOTE=1 to build libdcurl.so and remote-worker
* make BUILD_REMOTE=1 check with RabbitMQ broker and remote-worker
* RPC with exclusive callback queues with TTL property
* AMQP connection management for multiple threads
* Implement local fallback PoW when remote interface fails

Related DLTcollab#137
@ajblane ajblane force-pushed the remote-interface branch from a084095 to 08e1cee Compare May 31, 2019 02:07
@marktwtn marktwtn merged commit e15af79 into DLTcollab:develop May 31, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Remote interface: Implement client side for RPC
4 participants