Skip to content

common/admin_socket: Increase socket timeouts#31623

Merged
tchaikov merged 1 commit intoceph:masterfrom
badone:wip-tracker-42387-extend-admin-socket-timeout
Nov 19, 2019
Merged

common/admin_socket: Increase socket timeouts#31623
tchaikov merged 1 commit intoceph:masterfrom
badone:wip-tracker-42387-extend-admin-socket-timeout

Conversation

@badone
Copy link
Contributor

@badone badone commented Nov 14, 2019

With the move of the 'bench' command to the admin socket the recv
timeout is being exceeded in testing due to the duration of the command.

Fixes: https://tracker.ceph.com/issues/42387

Signed-off-by: Brad Hubbard bhubbard@redhat.com

Checklist

  • References tracker ticket
  • Updates documentation if necessary
  • Includes tests for new functionality or reproducer for bug

Show available Jenkins commands
  • jenkins retest this please
  • jenkins test crimson perf
  • jenkins test signed
  • jenkins test make check
  • jenkins test make check arm64
  • jenkins test submodules
  • jenkins test dashboard
  • jenkins test dashboard backend
  • jenkins test docs
  • jenkins render docs
  • jenkins test ceph-volume all
  • jenkins test ceph-volume tox

With the move of the 'bench' command to the admin socket the recv
timeout is being exceeded in testing due to the duration of the command.

Fixes: https://tracker.ceph.com/issues/42387

Signed-off-by: Brad Hubbard <bhubbard@redhat.com>
@badone
Copy link
Contributor Author

badone commented Nov 14, 2019

Since this is an arbitrary limit I'm looking for feedback as to whether this is a sufficient "fix" From my testing the 'bench' command does not exceed 10 seconds but some future command may?

@liewegas
Copy link
Member

It is a bit arbitrary... but as long as it's > the default bench duration it's probably fine?

@badone
Copy link
Contributor Author

badone commented Nov 15, 2019

@liewegas the default bench duration is dependant on the speed of the storage (by default we write bench count 1073741824 bsize 4 MiB (default count=1G default size=4MB)). I can break it with the new timeout values by sending '{"prefix":"bench", "count": 2147483648}' to the admin socket. We could set SO_RCVTIMEO to zero and disable timeouts and accept what comes with that or we could set a fair bit larger value and hope no one ever hits it?

@badone
Copy link
Contributor Author

badone commented Nov 15, 2019

Of course we could also make it user-configurable (which might be wise).

@badone
Copy link
Contributor Author

badone commented Nov 16, 2019

@liewegas If you're happy with this as it stands (based on our IRC conversation) could you ACK it?

Copy link
Member

@liewegas liewegas left a comment

Choose a reason for hiding this comment

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

This change seems fine on its face.. the timeout is arbitrary. We should keep in mind that the bench command could be improved in the future to not block the asok thread, though. I'm not sure it really matters given it's not something you'd normally do to a production system.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants