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

Rename max_write_buffer_size in _ccr/stats as it's misleading #33906

Closed
dliappis opened this issue Sep 20, 2018 · 6 comments
Closed

Rename max_write_buffer_size in _ccr/stats as it's misleading #33906

dliappis opened this issue Sep 20, 2018 · 6 comments
Assignees
Labels
:Distributed Indexing/CCR Issues around the Cross Cluster State Replication features

Comments

@dliappis
Copy link
Contributor

The max_write_buffer_size parameter name is misleading: it gives the impression that you are controlling directly the size of the allocated write buffer per shard, however it is a threshold controlling the amount of subsequent reads.

For example with the following index settings:

{
  "max_concurrent_read_batches": 1,
  "max_concurrent_write_batches": 1,
  "max_batch_operation_count": 32768,
  "max_batch_size_in_bytes": 4718592,
  "max_write_buffer_size": 33000
}

it's possible that one read brings max_batch_operation_count 32768 # of docs, the subsequent read sees that max_write_buffer_size is still higher (33000) and retrieves another batch of 32768 documents. In this case and instance, the _ccr/stats field number_of_queued_writes correctly reports 65535.

One suggestion would be to rename:
max_write_buffer_size -> max_subsequent_reads_into_write_buffer or replace subsequent with next to make it shorter. Any other suggestion is welcome of course.

@dliappis dliappis added the :Distributed Indexing/CCR Issues around the Cross Cluster State Replication features label Sep 20, 2018
@elasticmachine
Copy link
Collaborator

Pinging @elastic/es-distributed

@dliappis
Copy link
Contributor Author

cc @jasontedor for awareness.

@bleskes
Copy link
Contributor

bleskes commented Sep 21, 2018

+1 to rename. I would propose max_queued_writes

@dliappis
Copy link
Contributor Author

@martijnvg As #34797 already renames max_write_buffer_size to max_write_buffer_count, I guess we can close #33906?

@martijnvg
Copy link
Member

@dliappis Yes, I think we can close this issue.

@dliappis
Copy link
Contributor Author

Closed as per #33906 (comment)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
:Distributed Indexing/CCR Issues around the Cross Cluster State Replication features
Projects
None yet
Development

No branches or pull requests

4 participants