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

ES memory write race fix and performance improvement (#67) #91

Merged
merged 1 commit into from
Feb 10, 2022

Conversation

paramite
Copy link
Member

@paramite paramite commented Feb 7, 2022

  • Fix Loki readiness check
  • Allow multiple API calls to ES
    • This patch adds possibility to submit multiple requests to ES in paralel
      and hence improving performance.
  • Revert "Fix concurrent conflicts in elasticsearch (Fix concurrent conflicts in elasticsearch #53)"
    • Unfortunately concurrent.Map is not ready for concurrency,
      so we need to avoid using it.
    • This reverts commit e61141a.
  • Fix concurrency in ES client message buffer
    • "fatal error: concurrent map writes"
      /go/src/github.com/infrawatch/sg-core/plugins/application/elasticsearch/main.go:95
      /go/src/github.com/infrawatch/sg-core/pkg/bus.go:65 calls Application callbacks as a goroutine
      which results in concurrency in ReceiveEvent(). I've added a mutex to protect access.
    • Co-authored-by: Chris Sibbitt csibbitt@redhat.com
  • Remove unused package
    • concurrent.Map is unused throughout the project and it did not work as expected,
      because values could be overwritten when unlocked after returning from method.
  • Add amqp1 transport
  • Don't transfer whole message through channels
    • This patch makes channel synchronization use pointers instead of whole messages,
      which improves performance of the elasticsearch app plugin. It also adds mechanism
      to flush index buffer in case of remnants of some batch messages are kept
      for longer time.
  • Integration CI fixes
    • simplify rsyslog configuration to work with newer rsyslog
    • stop using ELN repo because of rsyslog-omamqp1
      (plugin is available in appstream)
    • add proper wait time between steps instead of simple sleep
  • Update .github/workflows/tests.yml
  • Move golint exclude-rules to proper place

(cherry picked from commit 7d46ba5)

@github-actions
Copy link

github-actions bot commented Feb 7, 2022

Coverage Status

leifmadsen
leifmadsen previously approved these changes Feb 9, 2022
* Fix Loki readiness check
* Allow multiple API calls to ES
  - This patch adds possibility to submit multiple requests to ES in paralel
     and hence improving performance.
* Revert "Fix concurrent conflicts in elasticsearch (#53)"
  - Unfortunately concurrent.Map is not ready for concurrency,
     so we need to avoid using it.
  - This reverts commit e61141a.
* Fix concurrency in ES client message buffer
  -  "fatal error: concurrent map writes"
      /go/src/github.com/infrawatch/sg-core/plugins/application/elasticsearch/main.go:95
     /go/src/github.com/infrawatch/sg-core/pkg/bus.go:65 calls Application callbacks as a goroutine
     which results in concurrency in ReceiveEvent(). I've added a mutex to protect access.
  - Co-authored-by: Chris Sibbitt <csibbitt@redhat.com>
* Remove unused package
  - concurrent.Map is unused throughout the project and it did not work as expected,
     because values could be overwritten when unlocked after returning from method.
* Add amqp1 transport
* Don't transfer whole message through channels
   - This patch makes channel synchronization use pointers instead of whole messages,
     which improves performance of the elasticsearch app plugin. It also adds mechanism
     to flush index buffer in case of remnants of some batch messages are kept
      for longer time.
* Integration CI fixes
  - simplify rsyslog configuration to work with newer rsyslog
  - stop using ELN repo because of rsyslog-omamqp1
     (plugin is available in appstream)
  - add proper wait time between steps instead of simple sleep
* Update .github/workflows/tests.yml
  - Co-authored-by: Leif Madsen <lmadsen@redhat.com>
* Move golint exclude-rules to proper place

(cherry picked from commit 7d46ba5)
@paramite
Copy link
Member Author

paramite commented Feb 9, 2022

Rebased on latest stable-1.3

@paramite paramite merged commit ac1983b into stable-1.3 Feb 10, 2022
@paramite paramite deleted the ci-fix-no.2-1.3 branch February 10, 2022 10:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

2 participants