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

Forward buffer #549

Merged
merged 4 commits into from
Feb 15, 2022
Merged

Forward buffer #549

merged 4 commits into from
Feb 15, 2022

Conversation

armstrmi
Copy link
Contributor

Description of Changes

  • Entries are now flushed upon the closing of buffer for forward output
  • Updated tests to verify forward output does send entries after closing

Please check that the PR fulfills these requirements

  • Tests for the changes have been added (for bug fixes / features)
  • Docs have been added / updated (for bug fixes / features)
  • Add a changelog entry (for non-trivial bug fixes / features)
  • CI passes

Copy link
Contributor

@cpheps cpheps left a comment

Choose a reason for hiding this comment

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

LGTM few small comments

operator/builtin/output/forward/forward.go Show resolved Hide resolved
operator/builtin/output/forward/forward.go Show resolved Hide resolved
@armstrmi
Copy link
Contributor Author

@BinaryFissionGames Looks good to you?

Copy link
Contributor

@BinaryFissionGames BinaryFissionGames left a comment

Choose a reason for hiding this comment

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

Just have a few small nitpicks with the test, then it should be good to merge.

Copy link
Contributor

@BinaryFissionGames BinaryFissionGames left a comment

Choose a reason for hiding this comment

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

LGTM!

Copy link
Contributor

@cpheps cpheps left a comment

Choose a reason for hiding this comment

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

LGTM

@cpheps
Copy link
Contributor

cpheps commented Feb 15, 2022

Test failures are unrelated to code changes.

@cpheps cpheps merged commit aa0b946 into buffer-rework Feb 15, 2022
@cpheps cpheps deleted the forward-buffer branch February 15, 2022 19:55
cpheps pushed a commit that referenced this pull request Feb 16, 2022
* push up forward output

* Refactored flush buffer

* Updated buffer close test for forward output

* Added closing of test server in forward tests

Signed-off-by: Corbin Phelps <[email protected]>

Co-authored-by: jmwilliams89 <[email protected]>
Co-authored-by: Corbin Phelps <[email protected]>
cpheps pushed a commit that referenced this pull request Feb 17, 2022
* push up forward output

* Refactored flush buffer

* Updated buffer close test for forward output

* Added closing of test server in forward tests

Signed-off-by: Corbin Phelps <[email protected]>

Co-authored-by: jmwilliams89 <[email protected]>
Co-authored-by: Corbin Phelps <[email protected]>
cpheps pushed a commit that referenced this pull request Feb 18, 2022
* push up forward output

* Refactored flush buffer

* Updated buffer close test for forward output

* Added closing of test server in forward tests

Signed-off-by: Corbin Phelps <[email protected]>

Co-authored-by: jmwilliams89 <[email protected]>
Co-authored-by: Corbin Phelps <[email protected]>
cpheps pushed a commit that referenced this pull request Feb 21, 2022
* push up forward output

* Refactored flush buffer

* Updated buffer close test for forward output

* Added closing of test server in forward tests

Signed-off-by: Corbin Phelps <[email protected]>

Co-authored-by: jmwilliams89 <[email protected]>
Co-authored-by: Corbin Phelps <[email protected]>
cpheps pushed a commit that referenced this pull request Feb 23, 2022
* push up forward output

* Refactored flush buffer

* Updated buffer close test for forward output

* Added closing of test server in forward tests

Signed-off-by: Corbin Phelps <[email protected]>

Co-authored-by: jmwilliams89 <[email protected]>
Co-authored-by: Corbin Phelps <[email protected]>
cpheps pushed a commit that referenced this pull request Feb 25, 2022
* push up forward output

* Refactored flush buffer

* Updated buffer close test for forward output

* Added closing of test server in forward tests

Signed-off-by: Corbin Phelps <[email protected]>

Co-authored-by: jmwilliams89 <[email protected]>
Co-authored-by: Corbin Phelps <[email protected]>
cpheps pushed a commit that referenced this pull request Feb 28, 2022
* push up forward output

* Refactored flush buffer

* Updated buffer close test for forward output

* Added closing of test server in forward tests

Signed-off-by: Corbin Phelps <[email protected]>

Co-authored-by: jmwilliams89 <[email protected]>
Co-authored-by: Corbin Phelps <[email protected]>
cpheps pushed a commit that referenced this pull request Mar 1, 2022
* push up forward output

* Refactored flush buffer

* Updated buffer close test for forward output

* Added closing of test server in forward tests

Signed-off-by: Corbin Phelps <[email protected]>

Co-authored-by: jmwilliams89 <[email protected]>
Co-authored-by: Corbin Phelps <[email protected]>
cpheps pushed a commit that referenced this pull request Mar 1, 2022
* Changed the buffer interface and updated affected operators

Moved current buffers to .old files and created stubs of new ones that satisfy the interface

Signed-off-by: Corbin Phelps <[email protected]>

* Refactored buffer interface to remove drain in favor of a close that returns entries (#498)

Signed-off-by: Corbin Phelps <[email protected]>

* Memory buffer rework (#497)

* Implemented reworked memory buffer and started tests

Signed-off-by: Corbin Phelps <[email protected]>

* Working on memory buffer tests

Signed-off-by: Corbin Phelps <[email protected]>

* Fully tested memory buffer

Signed-off-by: Corbin Phelps <[email protected]>

* Removed memory_buffer*.old files

Signed-off-by: Corbin Phelps <[email protected]>

* Removed intentional race condition from memory buffer test so race flag wouldn't tirgger

Signed-off-by: Corbin Phelps <[email protected]>

* Fixed comment in memorybuffer build

Signed-off-by: Corbin Phelps <[email protected]>

* Renamed buffer closed error

Signed-off-by: Corbin Phelps <[email protected]>

* Fixed output operators. Most would fail on error equality to a wrapped error

Signed-off-by: Corbin Phelps <[email protected]>

* Fixed comment for memorybuffer

Signed-off-by: Corbin Phelps <[email protected]>

* Removed operatorID from buffer builder interface (#499)

Signed-off-by: Corbin Phelps <[email protected]>

* Corrected documentation of memory buffer (#502)

* Corrected documentation of memory buffer

Signed-off-by: Corbin Phelps <[email protected]>

* Implemented PR feedback

Signed-off-by: Corbin Phelps <[email protected]>

* Implemented some wording changes

Signed-off-by: Corbin Phelps <[email protected]>

* Refactored flusher logic (#501)

* Refactored flusher to be a bit more readable and to have the context work as expected

Signed-off-by: Corbin Phelps <[email protected]>

* Cleaned up flusher doc

Signed-off-by: Corbin Phelps <[email protected]>

* Fixed up some comments and readme verbage

Signed-off-by: Corbin Phelps <[email protected]>

* Grammer PR feedback

Signed-off-by: Corbin Phelps <[email protected]>

* Disk buffer rewrite (#539)

* Ran go mod tidy

Signed-off-by: Corbin Phelps <[email protected]>

* Disk buffer rewrite

* cleanup some code; bump up test coverage a bit.

* Factor ErrEntryTooLarge into its own exported error

* fix mutex copy in tests

* use require.ErrorIs when asserting error type

* tweak doc

* Rewrite to use a circular buffer (ring buffer) -- in-progress

* add old benchmarks in

* Try using previous greedy strategy

The old disk buffer uses an "unfair" strategy, where
one reader will get all entries up to max chunk size before it times out.

* Use 1 mebibyte for disk buffer benchmark

* some performance changes

* fix broken circular file

* cleanup some todos and commented code

* multierr in disk_metadata constructor

* update path description

* don't export things other than the buffer

* rb -> cf, unexport some methods on circularFile struct

* unexport another method on circularFile

* Comment circular_file liberally

* remove multierr, minor comments

* unexport some more methods/structs

* remove redundant seekedRead sets in discard

* update docs; change marshalEntry to not take in a byte slice

* remove ability to partial write

* Capitalize struct methods used in other files

* Make Start, End etc in circular file unexported

* put cirularFile reads/writes into a for loop

* refactor some conditionals with small helper functions

Co-authored-by: Corbin Phelps <[email protected]>

* Forward buffer (#549)

* push up forward output

* Refactored flush buffer

* Updated buffer close test for forward output

* Added closing of test server in forward tests

Signed-off-by: Corbin Phelps <[email protected]>

Co-authored-by: jmwilliams89 <[email protected]>
Co-authored-by: Corbin Phelps <[email protected]>

* Elastic buffer (#552)

* Changed the buffer interface and updated affected operators

Moved current buffers to .old files and created stubs of new ones that satisfy the interface

Signed-off-by: Corbin Phelps <[email protected]>

* Refactored buffer interface to remove drain in favor of a close that returns entries (#498)

Signed-off-by: Corbin Phelps <[email protected]>

* Memory buffer rework (#497)

* Implemented reworked memory buffer and started tests

Signed-off-by: Corbin Phelps <[email protected]>

* Working on memory buffer tests

Signed-off-by: Corbin Phelps <[email protected]>

* Fully tested memory buffer

Signed-off-by: Corbin Phelps <[email protected]>

* Removed memory_buffer*.old files

Signed-off-by: Corbin Phelps <[email protected]>

* Removed intentional race condition from memory buffer test so race flag wouldn't tirgger

Signed-off-by: Corbin Phelps <[email protected]>

* Fixed comment in memorybuffer build

Signed-off-by: Corbin Phelps <[email protected]>

* Renamed buffer closed error

Signed-off-by: Corbin Phelps <[email protected]>

* Fixed output operators. Most would fail on error equality to a wrapped error

Signed-off-by: Corbin Phelps <[email protected]>

* Fixed comment for memorybuffer

Signed-off-by: Corbin Phelps <[email protected]>

* Removed operatorID from buffer builder interface (#499)

Signed-off-by: Corbin Phelps <[email protected]>

* Corrected documentation of memory buffer (#502)

* Corrected documentation of memory buffer

Signed-off-by: Corbin Phelps <[email protected]>

* Implemented PR feedback

Signed-off-by: Corbin Phelps <[email protected]>

* Implemented some wording changes

Signed-off-by: Corbin Phelps <[email protected]>

* Refactored flusher logic (#501)

* Refactored flusher to be a bit more readable and to have the context work as expected

Signed-off-by: Corbin Phelps <[email protected]>

* Cleaned up flusher doc

Signed-off-by: Corbin Phelps <[email protected]>

* Fixed up some comments and readme verbage

Signed-off-by: Corbin Phelps <[email protected]>

* Grammer PR feedback

Signed-off-by: Corbin Phelps <[email protected]>

* Disk buffer rewrite (#539)

* Ran go mod tidy

Signed-off-by: Corbin Phelps <[email protected]>

* Disk buffer rewrite

* cleanup some code; bump up test coverage a bit.

* Factor ErrEntryTooLarge into its own exported error

* fix mutex copy in tests

* use require.ErrorIs when asserting error type

* tweak doc

* Rewrite to use a circular buffer (ring buffer) -- in-progress

* add old benchmarks in

* Try using previous greedy strategy

The old disk buffer uses an "unfair" strategy, where
one reader will get all entries up to max chunk size before it times out.

* Use 1 mebibyte for disk buffer benchmark

* some performance changes

* fix broken circular file

* cleanup some todos and commented code

* multierr in disk_metadata constructor

* update path description

* don't export things other than the buffer

* rb -> cf, unexport some methods on circularFile struct

* unexport another method on circularFile

* Comment circular_file liberally

* remove multierr, minor comments

* unexport some more methods/structs

* remove redundant seekedRead sets in discard

* update docs; change marshalEntry to not take in a byte slice

* remove ability to partial write

* Capitalize struct methods used in other files

* Make Start, End etc in circular file unexported

* put cirularFile reads/writes into a for loop

* refactor some conditionals with small helper functions

Co-authored-by: Corbin Phelps <[email protected]>

* push up forward output

* Refactored flush buffer

* Updated buffer close test for forward output

* push up forward output

* Refactored flush buffer

* Updated buffer close test for forward output

* updated elastic output

* Changed the buffer interface and updated affected operators

Moved current buffers to .old files and created stubs of new ones that satisfy the interface

Signed-off-by: Corbin Phelps <[email protected]>

* Memory buffer rework (#497)

* Implemented reworked memory buffer and started tests

Signed-off-by: Corbin Phelps <[email protected]>

* Working on memory buffer tests

Signed-off-by: Corbin Phelps <[email protected]>

* Fully tested memory buffer

Signed-off-by: Corbin Phelps <[email protected]>

* Removed memory_buffer*.old files

Signed-off-by: Corbin Phelps <[email protected]>

* Removed intentional race condition from memory buffer test so race flag wouldn't tirgger

Signed-off-by: Corbin Phelps <[email protected]>

* Fixed comment in memorybuffer build

Signed-off-by: Corbin Phelps <[email protected]>

* Renamed buffer closed error

Signed-off-by: Corbin Phelps <[email protected]>

* Fixed output operators. Most would fail on error equality to a wrapped error

Signed-off-by: Corbin Phelps <[email protected]>

* Fixed comment for memorybuffer

Signed-off-by: Corbin Phelps <[email protected]>

* Removed operatorID from buffer builder interface (#499)

Signed-off-by: Corbin Phelps <[email protected]>

* Disk buffer rewrite (#539)

* Ran go mod tidy

Signed-off-by: Corbin Phelps <[email protected]>

* Disk buffer rewrite

* cleanup some code; bump up test coverage a bit.

* Factor ErrEntryTooLarge into its own exported error

* fix mutex copy in tests

* use require.ErrorIs when asserting error type

* tweak doc

* Rewrite to use a circular buffer (ring buffer) -- in-progress

* add old benchmarks in

* Try using previous greedy strategy

The old disk buffer uses an "unfair" strategy, where
one reader will get all entries up to max chunk size before it times out.

* Use 1 mebibyte for disk buffer benchmark

* some performance changes

* fix broken circular file

* cleanup some todos and commented code

* multierr in disk_metadata constructor

* update path description

* don't export things other than the buffer

* rb -> cf, unexport some methods on circularFile struct

* unexport another method on circularFile

* Comment circular_file liberally

* remove multierr, minor comments

* unexport some more methods/structs

* remove redundant seekedRead sets in discard

* update docs; change marshalEntry to not take in a byte slice

* remove ability to partial write

* Capitalize struct methods used in other files

* Make Start, End etc in circular file unexported

* put cirularFile reads/writes into a for loop

* refactor some conditionals with small helper functions

Co-authored-by: Corbin Phelps <[email protected]>

* got rid of errorf in stop()

Co-authored-by: Corbin Phelps <[email protected]>
Co-authored-by: Corbin Phelps <[email protected]>
Co-authored-by: Brandon Johnson <[email protected]>
Co-authored-by: jmwilliams89 <[email protected]>

* Google buffer (#563)

* wip

* wip

* flushes buffer after Stop()

* updated google-output tests

* made recommended changes from brandon

* Linter fixes (#566)

Signed-off-by: Corbin Phelps <[email protected]>

* added newrelic close buffer (#577)

* added newrelic close buffer

* made necessary fixes

* made necessary fixes

* unexport client from client.go

* Fixed reverted timeout in disk test

Signed-off-by: Corbin Phelps <[email protected]>

* added check for no entries upon stop

Co-authored-by: Brandon Johnson <[email protected]>
Co-authored-by: Mitchell Armstrong <[email protected]>
Co-authored-by: jmwilliams89 <[email protected]>
Co-authored-by: armstrmi <[email protected]>
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.

4 participants