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

AbstractConfigurableMongoDbMessageStore's getNextId might overflow to long leading to "java.lang.Long cannot be cast to java.lang.Integer" #3336

Closed
juanavelez opened this issue Jul 13, 2020 · 8 comments · Fixed by #3385
Assignees
Milestone

Comments

@juanavelez
Copy link

In what version(s) of Spring Integration are you seeing this issue?

It happens from version 4.3.12.RELEASE up to current. But it is possible that it happens with prior/future versions as well.

Describe the bug

Because getNextId performs an "increment" (by 1) on the "sequence" field of the special "messagesSequence" document, after Integer.MAX_VALUE calls to getNextId, next value will overflow from Integer.MAX_VALUE to a Long and when trying to retrieve it, will get fail with:

Caused by: java.lang.ClassCastException: java.lang.Long cannot be cast to java.lang.Integer

	/**
	 * Perform MongoDB {@code INC} operation for the document, which contains the {@link MessageDocument}
	 * {@code sequence}, and return the new incremented value for the new {@link MessageDocument}.
	 * The {@link #SEQUENCE_NAME} document is created on demand.
	 * @return the next sequence value.
	 */
	protected int getNextId() {
		Query query = Query.query(Criteria.where("_id").is(SEQUENCE_NAME));
		query.fields().include(MessageDocumentFields.SEQUENCE);
		return (Integer) this.mongoTemplate.findAndModify(query,
				new Update().inc(MessageDocumentFields.SEQUENCE, 1),
				FindAndModifyOptions.options().returnNew(true).upsert(true),
				Map.class, this.collectionName)
				.get(MessageDocumentFields.SEQUENCE);
	}

To Reproduce

Add Integer.MAX_VALUE + 1 of messages in aggregation store.

Caused by: java.lang.ClassCastException: java.lang.Long cannot be cast to java.lang.Integer
	at org.springframework.integration.mongodb.store.AbstractConfigurableMongoDbMessageStore.getNextId(AbstractConfigurableMongoDbMessageStore.java:180)
	at org.springframework.integration.mongodb.store.ConfigurableMongoDbMessageStore.addMessagesToGroup(ConfigurableMongoDbMessageStore.java:196)
	at org.springframework.integration.mongodb.store.ConfigurableMongoDbMessageStore.addMessageToGroup(ConfigurableMongoDbMessageStore.java:167)
	at org.springframework.integration.aggregator.AbstractCorrelatingMessageHandler.store(AbstractCorrelatingMessageHandler.java:621)
	at org.springframework.integration.aggregator.AbstractCorrelatingMessageHandler.handleMessageInternal(AbstractCorrelatingMessageHandler.java:413)
	at org.springframework.integration.handler.AbstractMessageHandler.handleMessage(AbstractMessageHandler.java:127)

Expected behavior

Well, getNextId returns an int which populates the MessageDocument's sequence which is an int, so most likely, the MessageDocument's sequence's type needs to be changed from int to long and then getNextId, needs to return a long.

@juanavelez juanavelez added status: waiting-for-triage The issue need to be evaluated and its future decided type: bug labels Jul 13, 2020
@garyrussell
Copy link
Contributor

Is this a real problem, or hypothetical?

@garyrussell garyrussell added the status: waiting-for-reporter Needs a feedback from the reporter label Jul 13, 2020
@artembilan
Copy link
Member

Does it work if you change it to long?
Probably it is not a big deal for end-users if that property is going to be changed to long...

We might not have it as back-ported to all supported versions because existing documents in DB might not be updated to a new type, but we definitely can consider it as a fix for the current version in progress.

Thanks

@garyrussell
Copy link
Contributor

But my question is:

Add Integer.MAX_VALUE + 1 of messages in aggregation store.

Is this even practical? How would such a message store perform?

@garyrussell
Copy link
Contributor

garyrussell commented Jul 13, 2020

Or is this just like a sequence - it doesn't mean there would be that number of messages concurrently (in which case, yes, it should be changed - but I would still expect it to take a loooong time to hit that limit).

@juanavelez
Copy link
Author

yes, this is a real problem. we use the aggregation store to "aggregate" millions of messages sent by our customers and then release when they are complete. Most messages are single-messages but because we process 30+ (and growing) million every day we started seeing these errors starting on Friday last week For now, we are resetting that field back to 0 in MongoDB.

@juanavelez
Copy link
Author

juanavelez commented Jul 13, 2020

Or is this just like a sequence - it doesn't mean there would be that number of messages concurrently (in which case, yes, it should be changed - but I would still expect it to take a loooong time to hit that limit).

yes it is a sequence that gets incremented with each message that ts "aggreagated". For us, it took several months to reach the problem and we have a workaround (reset in MongoDB to 0) but we'd like a long term solution.

@juanavelez
Copy link
Author

A clarification: The amount of messages that get aggregated are never beyond of the hundreds, and this issue is not related to the amount of messages in a group. The issue here is that each time a message is "aggregated", there is a mongodb-based sequence that gets incremented. You'd think that 2^31-1 would be a lot of messages to aggregate (and they are indeed a lot) but we process that much in weeks, so the use case is applicable and valid at least for us.

@artembilan
Copy link
Member

So, yeah... Let's go ahead and fix the issue changing from int to long!
Looks like Java long is mapped to proper MongoDB type:

if (source instanceof Integer) {
	return new BsonInt32((Integer) source);
}

if (source instanceof Long) {
	return new BsonInt64((Long) source);
}

@artembilan artembilan added in: mongodb and removed status: waiting-for-triage The issue need to be evaluated and its future decided labels Jul 15, 2020
@artembilan artembilan added this to the 5.4.x milestone Jul 15, 2020
@artembilan artembilan modified the milestones: 5.4.x, 5.4 M3 Sep 14, 2020
@artembilan artembilan removed the status: waiting-for-reporter Needs a feedback from the reporter label Sep 14, 2020
@artembilan artembilan self-assigned this Sep 14, 2020
artembilan added a commit to artembilan/spring-integration that referenced this issue Sep 14, 2020
Fixes spring-projects#3336

Turns out there are some scenarios where too many messages
are transferred through the message store, so `int` for
sequence is not enough as a type

* Change sequence to `long` to widen a sequence lifespan
garyrussell pushed a commit that referenced this issue Sep 15, 2020
* GH-3336: Change MongoDb Store sequence to long

Fixes #3336

Turns out there are some scenarios where too many messages
are transferred through the message store, so `int` for
sequence is not enough as a type

* Change sequence to `long` to widen a sequence lifespan

* * Change MongoDb store to deal with `Number.longValue()`
instead of casting which doesn't work from `Integer` to `Long`.
This way we can keep an old sequence document with an `int`
type for value
* Documents with new `long` type for their sequence field are OK.
The `NumberToNumberConverter` has an effect converting `int` to `long`
properly.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants