Split events emitted at once to multi chunks #1062

tagomoris · 2016-06-20T14:12:33Z

This change make it possible to emit large event stream into some chunks.
It can't be done at v0.10/v0.12, but it should be done to remove warnings about chunks larger than chunk limit size.

When I was writing this change, I found some problems about locking chunks and buffer global lock.
I also improved it because the buffer with this change will operate much more chunks than ever.

tagomoris · 2016-06-20T14:13:24Z

@repeatedly Please review this change!

repeatedly · 2016-06-21T08:15:49Z

@sonots Could you review this change first?

repeatedly · 2016-06-21T09:22:23Z

BTW, tests are filed on Mac environment. Is this known issue?

repeatedly · 2016-06-21T09:30:57Z

lib/fluent/event.rb

@@ -70,6 +88,10 @@ def repeatable?
      true
    end

+    def slice(index, num)
+      self.dup


If slice(1, 1) is called, slice should return same content, not empty stream?

I can't understand what you mean.
But, I think that OneEventStream#slice should return empty stream for first argument >=1.
I'll fix this code so.

tagomoris · 2016-06-22T06:55:05Z

@repeatedly That's a know one, I'm investigating it continuously.

tagomoris · 2016-06-23T06:21:11Z

lib/fluent/plugin/buffer.rb

+        if splits_count > data.size
+          splits_count = data.size
+        end
+        slice_size = if splits_count > data.size


The first condition is nonsense.

tagomoris · 2016-06-27T04:25:49Z

I pushed some commits to fixes along with some comments on this thread, and rebased on current master HEAD.

tagomoris · 2016-06-27T07:48:34Z

AppVeyor's one task is still failing (maybe file handle leak or something else), but I'll merge this later.
@repeatedly Any other review comments?

repeatedly · 2016-06-27T07:53:08Z

I need more time to review multi-threaded part and benchmark.
For safety v0.14.1 release, merging this patch is better in v0.14.2.

tagomoris · 2016-07-01T07:53:29Z

Memo: I found that this change miss to update Fluent::Compat::BufferedOutput#handle_stream_simple.

tagomoris · 2016-07-04T02:36:58Z

I rebased changes on master HEAD, and added a fix for compat layer.

tagomoris · 2016-07-04T02:37:41Z

@repeatedly Please review the changes. I'll take a look for test failures in osx environments.

repeatedly · 2016-07-07T11:37:51Z

lib/fluent/event.rb

+      end
+      # @size should be updated always right after unpack.
+      # The real size of unpacked objects are correct, rather than given size.
+      @size = @unpacked_times.size


After unpacked, @data should be set to nil for GC?

Hmm... @data is used in empty?.

@data should be kept as is, because #to_msgpack_stream returns @data itself.

tagomoris · 2016-07-11T07:35:52Z

@repeatedly Please check this change with latest commits.

tagomoris · 2016-07-12T10:31:24Z

@repeatedly ping?

repeatedly · 2016-07-13T07:46:49Z

lib/fluent/plugin/buffer.rb

          end
        end

        unless stored
          # try step-by-step appending if data can't be stored into existing a chunk in non-bulk mode
-          write_step_by_step(metadata, data, data.size / 3, &block)
+          write_step_by_step(metadata, data, format, 10, &block)


Is this 10 heuristic value?
If so, comment is good for why we choose 10.

I added a comment for it.

…BufferChunkOverflowError

To implement it efficiently, caching unpacked objects were introduced in MessagePackEventStream. It make it possible to slice, iterate and duplicate it efficiently after it was iterated or #size called at once.

… in write_step_by_step

by split-and-join event streams. With this change, locking/releasing was improved at the same time.

* unstaged chunks will be enqueued, so there are no reason to add these sizes to staged bytes * only when unstaged chunk is staged, its size should be added

tagomoris · 2016-07-20T05:08:34Z

@repeatedly I added a commit to add code comment. Is that all for your review comment?

repeatedly · 2016-08-08T17:35:51Z

I tested split case CPU usage on 2 c3.8xlarge instances.
Here is the result with 100k / sec.
From the result, if the incoming events are larger than buffer_chunk_limit, CPU usage is high.

fluentd-benchmark one_forward based configuration

flush_interval 0s. No split chunks.

Forwarder CPU: 78%
Aggregator CPU: 15%

Baseline: v0.12.27

Fowarder CPU: 69%
Aggregator CPU: 10.5%

Use tdlog instead of flowcounter_simple for buffer test

Forwarder (buffer_chunk_limit 32m)

CPU: 72%

Aggregator (tdlog + buffer_chunk_limit 65m)

No split chunks because aggregator's buffer_chunk_limit is larger than forwarder's chunk.

CPU: 45% - 70%

Aggregator (tdlog + buffer_chunk_limit 8m)

Split 32mb chunks into smaller chunks.

CPU: 65% - 85% (average 75%)

tagomoris · 2016-08-08T18:55:10Z

It is in my design - we can't help to consume CPU usage for splitting a chunk into chunks. We get safer chunking in exchange for additional CPU usage.
@repeatedly How do you think about it?

repeatedly · 2016-08-08T19:24:18Z

@tagomoris Yeah, hard to reduce CPU usage for it.
But we should mention CPU usage issue in the announcement and fluentd-docs because
it may degrade the pipeline performance on aggregator node.

tagomoris · 2016-08-08T19:30:04Z

I agree about it.

tagomoris added the v0.14 label Jun 20, 2016

tagomoris force-pushed the split-events-emitted-at-once-to-multi-chunks branch from e960f94 to 8985883 Compare June 21, 2016 01:39

repeatedly reviewed Jun 21, 2016
View reviewed changes

This was referenced Jun 22, 2016

Plugin helper tag time hostname #1063

Merged

Migrate out_copy and out_roundrobin to v0.14 API #1064

Merged

tagomoris reviewed Jun 23, 2016
View reviewed changes

tagomoris force-pushed the split-events-emitted-at-once-to-multi-chunks branch from 8985883 to 9ae7d1b Compare June 27, 2016 04:25

tagomoris force-pushed the split-events-emitted-at-once-to-multi-chunks branch from 9ae7d1b to 5e051e2 Compare July 4, 2016 02:13

tagomoris force-pushed the split-events-emitted-at-once-to-multi-chunks branch from 14c7719 to 164fa09 Compare July 4, 2016 05:50

repeatedly reviewed Jul 7, 2016
View reviewed changes

repeatedly reviewed Jul 13, 2016
View reviewed changes

tagomoris added 16 commits July 19, 2016 15:11

changing internal API to make it possible to split event streams for …

a353a36

…BufferChunkOverflowError

Add #slice and correct #size method to classes of event stream.

55eda5f

To implement it efficiently, caching unpacked objects were introduced in MessagePackEventStream. It make it possible to slice, iterate and duplicate it efficiently after it was iterated or #size called at once.

add for mocking

0206472

use same proc instances for mocking

c46b966

add status "unstaged" for chunks generated but not staged immediately…

0012d46

… in write_step_by_step

Large event streams from an emit will be split into multi chunks

b5f2e9f

by split-and-join event streams. With this change, locking/releasing was improved at the same time.

fix with updated status control

3fcd1fa

fix tests with updated buffer internal

d7a1128

remove useless statement

a1285cf

add optional argument to pass size of es (mainly for compat layer)

94c3f4b

fix message to show where the bug is

e2bd8aa

Fix bug about stage size calculation

5ecaca7

* unstaged chunks will be enqueued, so there are no reason to add these sizes to staged bytes * only when unstaged chunk is staged, its size should be added

rename variable for ease to understand

ceb4723

delete definition wrongly added when rebased

1fe55f7

rename to reduce warning

433b34c

add comment about splitting event stream to write into buffer chunks

fe3a860

tagomoris force-pushed the split-events-emitted-at-once-to-multi-chunks branch from 136678b to fe3a860 Compare July 19, 2016 06:25

tagomoris merged commit 19b6e05 into master Jul 20, 2016

tagomoris deleted the split-events-emitted-at-once-to-multi-chunks branch July 20, 2016 07:59

repeatedly mentioned this pull request Jan 31, 2017

in_forward ingests and saves buffered files larger than buffer_chunk_limit #1436

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Split events emitted at once to multi chunks #1062

Split events emitted at once to multi chunks #1062

tagomoris commented Jun 20, 2016

tagomoris commented Jun 20, 2016

repeatedly commented Jun 21, 2016

repeatedly commented Jun 21, 2016

repeatedly Jun 21, 2016 •

edited

Loading

tagomoris Jun 22, 2016 •

edited

Loading

tagomoris commented Jun 22, 2016

tagomoris Jun 23, 2016

tagomoris commented Jun 27, 2016

tagomoris commented Jun 27, 2016

repeatedly commented Jun 27, 2016

tagomoris commented Jul 1, 2016

tagomoris commented Jul 4, 2016

tagomoris commented Jul 4, 2016

repeatedly Jul 7, 2016

repeatedly Jul 7, 2016 •

edited

Loading

tagomoris Jul 8, 2016

tagomoris commented Jul 11, 2016

tagomoris commented Jul 12, 2016

repeatedly Jul 13, 2016 •

edited

Loading

tagomoris Jul 19, 2016

tagomoris commented Jul 20, 2016

repeatedly commented Aug 8, 2016

tagomoris commented Aug 8, 2016 •

edited

Loading

repeatedly commented Aug 8, 2016

tagomoris commented Aug 8, 2016

Split events emitted at once to multi chunks #1062

Split events emitted at once to multi chunks #1062

Conversation

tagomoris commented Jun 20, 2016

tagomoris commented Jun 20, 2016

repeatedly commented Jun 21, 2016

repeatedly commented Jun 21, 2016

repeatedly Jun 21, 2016 • edited Loading

Choose a reason for hiding this comment

tagomoris Jun 22, 2016 • edited Loading

Choose a reason for hiding this comment

tagomoris commented Jun 22, 2016

tagomoris Jun 23, 2016

Choose a reason for hiding this comment

tagomoris commented Jun 27, 2016

tagomoris commented Jun 27, 2016

repeatedly commented Jun 27, 2016

tagomoris commented Jul 1, 2016

tagomoris commented Jul 4, 2016

tagomoris commented Jul 4, 2016

repeatedly Jul 7, 2016

Choose a reason for hiding this comment

repeatedly Jul 7, 2016 • edited Loading

Choose a reason for hiding this comment

tagomoris Jul 8, 2016

Choose a reason for hiding this comment

tagomoris commented Jul 11, 2016

tagomoris commented Jul 12, 2016

repeatedly Jul 13, 2016 • edited Loading

Choose a reason for hiding this comment

tagomoris Jul 19, 2016

Choose a reason for hiding this comment

tagomoris commented Jul 20, 2016

repeatedly commented Aug 8, 2016

fluentd-benchmark one_forward based configuration

Baseline: v0.12.27

Use tdlog instead of flowcounter_simple for buffer test

Forwarder (buffer_chunk_limit 32m)

Aggregator (tdlog + buffer_chunk_limit 65m)

Aggregator (tdlog + buffer_chunk_limit 8m)

tagomoris commented Aug 8, 2016 • edited Loading

repeatedly commented Aug 8, 2016

tagomoris commented Aug 8, 2016

repeatedly Jun 21, 2016 •

edited

Loading

tagomoris Jun 22, 2016 •

edited

Loading

repeatedly Jul 7, 2016 •

edited

Loading

repeatedly Jul 13, 2016 •

edited

Loading

tagomoris commented Aug 8, 2016 •

edited

Loading