Skip to content

new concurrency model :single #96

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.

Sign up for GitHub

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

Closed
wants to merge 5 commits into from
Closed

new concurrency model :single #96

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
a1f3b6d
new concurrency model :shared
ph Aug 29, 2016
a8dd189
Uses :singles as the concurrency model
ph Sep 8, 2016
b5b3803
Update to 3.2.0
ph Sep 8, 2016
c3fc9f2
Delete test files on the remote bucket
ph Sep 8, 2016
808b6e0
fix typo in the changelog
ph Sep 8, 2016
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 7 additions & 0 deletions CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,13 @@
## 3.2.0
- Move to the new concurrency model `:single`
- use correct license identifier #99
- add support for `bucket_owner_full_control` in the canned ACL #87
- delete the test file but ignore any errors, because we actually only need to be able to write to S3. #97

## 3.1.2
- Fix improper shutdown of output worker threads
- improve exception handling

## 3.0.1
- Republish all the gems under jruby.

Expand Down
13 changes: 11 additions & 2 deletions lib/logstash/outputs/s3.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -79,6 +79,8 @@ class LogStash::Outputs::S3 < LogStash::Outputs::Base
config_name "s3"
default :codec, 'line'

concurrency :single
Copy link
Member

Choose a reason for hiding this comment

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

humm the PR name is "new concurrency model :shared" and the description says the plugin threadsafe, however this sets the model to :single? this means that 1 instance is shared among all threads and access to it is globally synchronized.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yewps, this plugin is in fact in this current state non threadsafe, I've made the plugin run for a few hours to discover that problem.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I will update the description


# S3 bucket
config :bucket, :validate => :string

Expand Down Expand Up @@ -214,8 +216,6 @@ def register
# http://ruby.awsblog.com/post/Tx16QY1CI5GVBFT/Threading-with-the-AWS-SDK-for-Ruby
AWS.eager_autoload!(AWS::S3)

workers_not_supported

@s3 = aws_s3_config
@upload_queue = Queue.new
@file_rotation_lock = Mutex.new
Expand Down Expand Up @@ -258,6 +258,15 @@ def test_s3_write

begin
write_on_bucket(test_filename)

begin
remote_filename = "#{@prefix}#{File.basename(test_filename)}"
bucket = @s3.buckets[@bucket]
bucket.objects[remote_filename].delete
rescue StandardError => e
# we actually only need `put_object`, but if we dont delete them
# we can have a lot of tests files
end
ensure
File.delete(test_filename)
end
Expand Down
2 changes: 1 addition & 1 deletion logstash-output-s3.gemspec
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
Gem::Specification.new do |s|

s.name = 'logstash-output-s3'
s.version = '3.1.2'
s.version = '3.2.0'
s.licenses = ['Apache-2.0']
s.summary = "This plugin was created for store the logstash's events into Amazon Simple Storage Service (Amazon S3)"
s.description = "This gem is a Logstash plugin required to be installed on top of the Logstash core pipeline using $LS_HOME/bin/logstash-plugin install gemname. This gem is not a stand-alone program"
Expand Down