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

Wait for required mappings to be available on the replica before indexing. #10786

Merged
merged 1 commit into from Apr 24, 2015

Conversation

jpountz
Copy link
Contributor

@jpountz jpountz commented Apr 24, 2015

Due to timing issues, mappings that are required to index a document might not
be available on the replica at indexing time. In that case the replica starts
listening to cluster state changes and re-parses the document until no dynamic
mappings updates are generated.

@jpountz
Copy link
Contributor Author

jpountz commented Apr 24, 2015

@s1monw I assigned it to you.

public String toString() {
try {
XContentBuilder builder = JsonXContent.contentBuilder();
toXContent(builder, new MapParams(Collections.<String, String>emptyMap()));
Copy link
Contributor

Choose a reason for hiding this comment

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

we have ToXContent.EMPTY_PARAMS

Copy link
Contributor

Choose a reason for hiding this comment

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

I really wonder if we should have Strings.toString(ToXcontent x) I think we have this code in a lot of places

Copy link
Contributor Author

Choose a reason for hiding this comment

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

+1

@s1monw
Copy link
Contributor

s1monw commented Apr 24, 2015

i left a bunch of comments, the only thing that I am concerned is not having a timeout... somehow we need to figure this out what to do if we pile up lots of requests? Maybe we can open a blocker issue to follow up on this?

@jpountz
Copy link
Contributor Author

jpountz commented Apr 24, 2015

I pushed a new commit. Regarding the timeout, you know this code better than me, I'm not as much aware of the consequences as you are. However @bleskes seemed to consider that having no timeout was better (please correct me if I misunderstood). For the record, having no timeout also allows to not call System.currentTimeMillis in ClusterStateObserver.

@s1monw
Copy link
Contributor

s1monw commented Apr 24, 2015

Looks good on my end, I think we should push as it is and open a followup to re-evaluate the approach. Makes sense?

…efore indexing.

Due to timing issues, mappings that are required to index a document might not
be available on the replica at indexing time. In that case the replica starts
listening to cluster state changes and re-parses the document until no dynamic
mappings updates are generated.
@jpountz
Copy link
Contributor Author

jpountz commented Apr 24, 2015

I opened #10797

jpountz added a commit that referenced this pull request Apr 24, 2015
Internal: Wait for required mappings to be available on the replica before indexing.
@jpountz jpountz merged commit 46ac32a into elastic:master Apr 24, 2015
@jpountz jpountz deleted the fix/dynamic_mappings_on_replicas branch April 24, 2015 20:23
@jpountz
Copy link
Contributor Author

jpountz commented Apr 28, 2015

For the record I tagged as non-issue since it's fixing a non released issue.

@clintongormley clintongormley added >enhancement :Search/Mapping Index mappings, including merging and defining field types and removed >non-issue labels Jun 8, 2015
@clintongormley clintongormley changed the title Internal: Wait for required mappings to be available on the replica before indexing. Wait for required mappings to be available on the replica before indexing. Jun 8, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
>enhancement :Search/Mapping Index mappings, including merging and defining field types v2.0.0-beta1
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants