Introduce server plugin helpers #1312

tagomoris · 2016-11-15T08:53:09Z

Server plugin helpers are to make plugin authors to write network servers very easily.
These make authors free from writing code to create sockets, handle event loops or threads.

The key features are:

callback-based network daemon execution (plugin helper style)
test driver support to wait startup/shutdown servers before actual test code running
creating sockets via socket manager in default for coming multi-process configuration support

The full features of socket/server plugin helpers and TBDs are:

TCP, UDP and TLS support (TLS will be implemented in future change)
TCP/TLS keepalive (in future)
IPv6 support

I committed all changes for this pull-request

server plugin helper: TCP/UDP implementation complete
tcp/udp input plugin: migrated to v0.14 APIs with server plugin helper
forward input plugin: updated with server plugin helper
tests of server plugin helper

To reduce the size of diff, keepalive support and socket plugin helper support will be implemented in following pull requests.

socket plugin helper and its tests
update of output plugins using sockets (including out_forward)
http input plugin update (in future with keepalive support)

tagomoris · 2016-11-15T08:54:03Z

@repeatedly Could you take a look for API design and basic features? If you are ok, I'll write tests and move forward to implement socket plugin helper.

repeatedly · 2016-11-16T00:15:04Z

lib/fluent/plugin/in_forward.rb

        router.emit(tag, time, record)
        option = msg[3]
      end

      # return option for response
      option
+    ensure
+      p(here: "ensure of on_message", error: $!) if $!


Why use p?

It's just about debugging. I missed to remote it.

repeatedly · 2016-11-16T04:03:25Z

lib/fluent/plugin_helper/server.rb

+      end
+
+      class TCPServerHandler < Coolio::TCPSocket
+        PEERADDR_FAILED = ["?", "?", "name resolusion failed", "?"]


No use in new implementation.

repeatedly · 2016-11-16T04:32:06Z

lib/fluent/plugin_helper/server.rb

+        end
+
+        def initialize(sock, resolve_name, max_bytes, flags, log, under_plugin_development, &callback)
+          raise ArgumentError, "socket is a UDPSocket" unless sock.is_a?(UDPSocket)


socket must be a UDPSocket: socket = #{sock} or socket is not a UDPSocket: socket = #{sock} is better.

repeatedly · 2016-11-16T06:00:39Z

Basic implementation looks good.

Off topic: How about moving under_plugin_development into Engine like Engine.under_plugin_development?
Passing under_plugin_development to each method seems bit messy...

repeatedly · 2016-11-16T06:10:38Z

lib/fluent/plugin_helper/server.rb

+        def on_read_without_connection(data)
+          @data_callback.call(data)
+        rescue => e
+          p(here: "error without connection", error: e)


repeatedly · 2016-11-16T06:10:43Z

lib/fluent/plugin_helper/server.rb

+        def on_read_with_connection(data)
+          @data_callback.call(data, self)
+        rescue => e
+          p(here: "error with connection", error: e)


tagomoris · 2016-11-16T09:22:21Z

@repeatedly No way. It's very bad practice to use Engine for such purpose. Engine is a singleton object, and using Engine for such purpose makes many messy code to overriding Engine.under_plugin_development? and to revert overriding in tests.

tagomoris · 2016-11-16T09:29:29Z

I pushed updated commits to follow review comments, and add some changes:

add Fluent::PluginHelper::Server::EventHandler module and added TCPServer/UDPServer under it
add Fluent::PluginHelper::Server::CallbackSocket and its subclasses of it for tcp/udp

I changed to pass an instance object of CallbackSocket to callbacks, to avoid to pass instances of cool.io classes. If plugin helper passes instances of cool.io classes, users may use methods of these classes (even if we want to keep these methods as semi-private).
For instances of original classes, we can control the variations of these methods (for compatibility).

tagomoris · 2016-11-16T09:38:45Z

There's no change about server plugin helper API for plugins in updated commits.

tagomoris · 2016-11-22T01:57:20Z

I pushed all commits to add/fix server plugin helper, and tests which pass on my laptop.
I'll check CI failure and fix it, but anyway, @repeatedly could you review this change?

repeatedly · 2016-11-22T07:07:03Z

lib/fluent/plugin/in_tcp.rb

    desc 'The payload is read up to this character.'
    config_param :delimiter, :string, default: "\n" # syslog family add "\n" to each message and this seems only way to split messages in tcp stream

-    def listen(callback)
-      log.info "listening tcp socket on #{@bind}:#{@port}"
+    def configure(conf)


should check source_host_key and source_hostname_key?

if !@source_host_key.nil? && @source_hostname_key.nil? @source_hostname_key = @source_host_key end

repeatedly · 2016-11-22T07:07:23Z

lib/fluent/plugin/in_tcp.rb

+              tag = extract_tag_from_record(record)
+              tag ||= @tag
+              time ||= extract_time_from_record(record) || Fluent::EventTime.now
+              record[@source_host_key] = conn.remote_host if @source_host_key


use @source_hostname_key instead.

repeatedly · 2016-11-22T07:09:56Z

lib/fluent/plugin/in_udp.rb

+            tag = extract_tag_from_record(record)
+            tag ||= @tag
+            time ||= extract_time_from_record(record) || Fluent::EventTime.now
+            record[@source_host_key] = sock.remote_host if @source_host_key


repeatedly · 2016-11-22T07:15:07Z

lib/fluent/plugin_helper/event_loop.rb


      attr_reader :_event_loop # for tests

      def event_loop_attach(watcher)
        @_event_loop_mutex.synchronize do
          @_event_loop.attach(watcher)
+          @_event_loop_attached_watchers << watcher


This means Loop#watchers sometimes returns incorrect watchers?

Loop#watchers contains watchers which are NOT registered by event loop helpers, like TCP connection I/O watchers registered by TCP server watcher.
These watchers should be detached/closed by these watchers itself. Detaching such watchers by event loop helpers forcedly makes troubles.

repeatedly · 2016-11-22T07:49:24Z

lib/fluent/plugin_helper/server.rb

+          when :data
+            @sock.data(&callback)
+          when :write_complete
+            cb = ->(){


Use one line for :close code.

repeatedly · 2016-11-22T07:54:37Z

lib/fluent/plugin_helper/server.rb

+          SOCK_OPT_FORMAT = 'I!I!' # { int l_onoff; int l_linger; }
+
+          def initialize(sock, close_callback, resolve_name, linger_timeout, log, under_plugin_development, connect_callback)
+            raise ArgumentError, "socket must be a TCPSocket" unless sock.is_a?(TCPSocket)


add : sock = #{sock} to show actual object.

…in shutdown

…hostname only for 127.0.0.1

…rameter)

…name)

tagomoris added the v0.14 label Nov 15, 2016

tagomoris assigned repeatedly Nov 15, 2016

tagomoris mentioned this pull request Nov 15, 2016

fix not to raise error when out_forward is initialized as secondary #1313

Merged

repeatedly reviewed Nov 16, 2016

View reviewed changes

tagomoris force-pushed the introduce-socket-server-plugin-helpers branch from 57e1c9f to a8a6ccc Compare November 16, 2016 09:15

tagomoris force-pushed the introduce-socket-server-plugin-helpers branch from a8a6ccc to fe38634 Compare November 16, 2016 09:24

tagomoris force-pushed the introduce-socket-server-plugin-helpers branch 3 times, most recently from a364153 to b6565b7 Compare November 21, 2016 11:49

tagomoris changed the title ~~[WIP] Introduce socket/server plugin helpers~~ [WIP] Introduce server plugin helpers Nov 21, 2016

tagomoris force-pushed the introduce-socket-server-plugin-helpers branch from b6565b7 to 722c9b6 Compare November 22, 2016 01:54

tagomoris changed the title ~~[WIP] Introduce server plugin helpers~~ Introduce server plugin helpers Nov 22, 2016

tagomoris force-pushed the introduce-socket-server-plugin-helpers branch from d563fa5 to aaaaa1c Compare November 22, 2016 06:52

repeatedly reviewed Nov 22, 2016

View reviewed changes

tagomoris force-pushed the introduce-socket-server-plugin-helpers branch from a7f83ce to 0e6c4ea Compare November 22, 2016 08:58

tagomoris added 26 commits November 28, 2016 15:33

update implementation using serever plugin helper

5c6ffce

fix buggy test code

9fb9c3d

On windows, @socket_manager_path is a port number

cca6e9a

close-on-exec is unavailable on windows

faa3137

fix not to set close-on-exec, because it's set by ruby

ebc8087

remove mixin not required anymore

6dca048

set do_not_reverse_lookup to sockets to listen and after accepted

45ac6a6

return watcher when attached, and detach watchers attached by plugin …

d4d5f35

…in shutdown

break #shutdown by timeout if any event watcher blocks infinitly

d766ecd

add argument checks

faa51a8

show protocol of created servers in dump

48fe5e6

remove all callbacks for udp server, add established connection handling

84d82b7

add tests of server plugin helper

93b5e00

show tests which be stuck

93e5c1e

update serverengine dependency to fix IPv6 support on Windows

3493bb9

fix not to use power_assert to avoid SystemStackError on this assertion

c12758e

fix to assert hostname directly for environments which does not have …

f29de3e

…hostname only for 127.0.0.1

update ServerEngine not to crash for IPv6 & Windows

5a97f75

refer both of source_hostname_key and source_host_key (as optional pa…

f1f5dc5

…rameter)

simplify code and inspect objects for errors

89ebb46

add helper method to get unused UDP ports

6497319

revert verbose testing option for CI services

742b79f

removed debugging message

cf763b9

separate setting socket options to share it with socket plugin helper

a29481f

rescue Errno::ECONNRESET for cases ICMP has trouble (for getting host…

ecc0959

…name)

require top level ServerEngine

4e63ef4

tagomoris force-pushed the introduce-socket-server-plugin-helpers branch from 08d2360 to 4e63ef4 Compare November 28, 2016 06:33

tagomoris merged commit c5c8709 into master Nov 28, 2016

cosmo0920 mentioned this pull request Nov 29, 2016

Added plugin tcp with ssl #1333

Closed

ganmacs deleted the introduce-socket-server-plugin-helpers branch July 11, 2019 07:59

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Introduce server plugin helpers #1312

Introduce server plugin helpers #1312

tagomoris commented Nov 15, 2016 •

edited

Loading

tagomoris commented Nov 15, 2016

repeatedly Nov 16, 2016

tagomoris Nov 16, 2016

repeatedly Nov 16, 2016

repeatedly Nov 16, 2016

repeatedly commented Nov 16, 2016

repeatedly Nov 16, 2016

repeatedly Nov 16, 2016

tagomoris commented Nov 16, 2016

tagomoris commented Nov 16, 2016

tagomoris commented Nov 16, 2016

tagomoris commented Nov 22, 2016

repeatedly Nov 22, 2016

tagomoris Nov 24, 2016

repeatedly Nov 22, 2016

repeatedly Nov 22, 2016

repeatedly Nov 22, 2016

tagomoris Nov 24, 2016

repeatedly Nov 22, 2016

repeatedly Nov 22, 2016

Introduce server plugin helpers #1312

Introduce server plugin helpers #1312

Conversation

tagomoris commented Nov 15, 2016 • edited Loading

tagomoris commented Nov 15, 2016

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

repeatedly commented Nov 16, 2016

Choose a reason for hiding this comment

Choose a reason for hiding this comment

tagomoris commented Nov 16, 2016

tagomoris commented Nov 16, 2016

tagomoris commented Nov 16, 2016

tagomoris commented Nov 22, 2016

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

tagomoris commented Nov 15, 2016 •

edited

Loading