You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@spark.apache.org by "jdesjean (via GitHub)" <gi...@apache.org> on 2023/08/17 23:58:08 UTC
[GitHub] [spark] jdesjean opened a new pull request, #42550: [SPARK-44861] jsonignore SparkListenerConnectOperationStarted.planRequest
jdesjean opened a new pull request, #42550:
URL: https://github.com/apache/spark/pull/42550
### What changes were proposed in this pull request?
Add @JsonIgnoreProperties({ "planRequest" }) to SparkListenerConnectOperationStarted.planRequest
### Why are the changes needed?
SparkListenerConnectOperationStarted was added as part of [SPARK-43923](https://issues.apache.org/jira/browse/SPARK-43923).
SparkListenerConnectOperationStarted.planRequest cannot be serialized & deserialized from json as it has recursive objects which causes failures when attempting these operations.
### Does this PR introduce _any_ user-facing change?
No
### How was this patch tested?
Unit
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] juliuszsompolski commented on a diff in pull request #42550: [SPARK-44861][CONNECT] jsonignore SparkListenerConnectOperationStarted.planRequest
Posted by "juliuszsompolski (via GitHub)" <gi...@apache.org>.
juliuszsompolski commented on code in PR #42550:
URL: https://github.com/apache/spark/pull/42550#discussion_r1298276465
##########
connector/connect/server/src/main/scala/org/apache/spark/sql/connect/service/ExecuteEventsManager.scala:
##########
@@ -278,6 +278,7 @@ case class ExecuteEventsManager(executeHolder: ExecuteHolder, clock: Clock) {
* @param extraTags:
* Additional metadata during the request.
*/
+@JsonIgnoreProperties({ "planRequest" })
Review Comment:
@gengliangwang @rednaxelafx does something need to be done so that history service defaults None on this planRequest missing from json, or will this just work?
Are there any suites where event log replay from json is tested where a test could be added for this?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] jdesjean commented on a diff in pull request #42550: [SPARK-44861][CONNECT] jsonignore SparkListenerConnectOperationStarted.planRequest
Posted by "jdesjean (via GitHub)" <gi...@apache.org>.
jdesjean commented on code in PR #42550:
URL: https://github.com/apache/spark/pull/42550#discussion_r1300686813
##########
connector/connect/server/src/main/scala/org/apache/spark/sql/connect/service/ExecuteEventsManager.scala:
##########
@@ -278,6 +278,7 @@ case class ExecuteEventsManager(executeHolder: ExecuteHolder, clock: Clock) {
* @param extraTags:
* Additional metadata during the request.
*/
+@JsonIgnoreProperties({ "planRequest" })
Review Comment:
I added json serde testing as done in https://github.com/apache/spark/pull/23728. The test showed that `@JsonIgnoreProperties({ "planRequest" })` didn't work on read.
```
[info] - SPARK-43923: post started *** FAILED *** (2 seconds, 220 milliseconds)
[info] com.fasterxml.jackson.databind.exc.InvalidDefinitionException: Cannot find a deserializer for non-concrete Collection type [collection type; class com.google.protobuf.ProtocolStringList, contains [simple type, class java.lang.String]]
[info] at [Source: (String)"{"Event":"org.apache.spark.sql.connect.service.SparkListenerConnectOperationStarted","jobTag":"SparkConnect_OperationTag_User_7f8102fa-64a6-4c6e-a2a1-d701de9c0dc3_Session_1_Operation_6a771009-bc23-4767-bef7-6f75cf92b3a0","operationId":"6a771009-bc23-4767-bef7-6f75cf92b3a0","eventTime":0,"sessionId":"7f8102fa-64a6-4c6e-a2a1-d701de9c0dc3","userId":"1","userName":"userName","statementText":"limit {\n limit: 10\n}\n","sparkSessionTags":[],"extraTags":{}}"; line: 1, column: 1]
[info] at com.fasterxml.jackson.databind.exc.InvalidDefinitionException.from(InvalidDefinitionException.java:67)
[info] at com.fasterxml.jackson.databind.DeserializationContext.reportBadDefinition(DeserializationContext.java:1915)
[info] at com.fasterxml.jackson.databind.deser.DeserializerCache._createAndCache2(DeserializerCache.java:268)
[info] at com.fasterxml.jackson.databind.deser.DeserializerCache._createAndCacheValueDeserializer(DeserializerCache.java:244)
[info] at com.fasterxml.jackson.databind.deser.DeserializerCache.findValueDeserializer(DeserializerCache.java:142)
[info] at com.fasterxml.jackson.databind.DeserializationContext.findNonContextualValueDeserializer(DeserializationContext.java:644)
[info] at com.fasterxml.jackson.databind.deser.BeanDeserializerBase.resolve(BeanDeserializerBase.java:539)
[info] at com.fasterxml.jackson.databind.deser.DeserializerCache._createAndCache2(DeserializerCache.java:294)
[info] at com.fasterxml.jackson.databind.deser.DeserializerCache._createAndCacheValueDeserializer(DeserializerCache.java:244)
[info] at com.fasterxml.jackson.databind.deser.DeserializerCache.findValueDeserializer(DeserializerCache.java:142)
[info] at com.fasterxml.jackson.databind.DeserializationContext.findContextualValueDeserializer(DeserializationContext.java:621)
[info] at com.fasterxml.jackson.module.scala.deser.OptionDeserializer.createContextual(OptionDeserializerModule.scala:50)
[info] at com.fasterxml.jackson.databind.DeserializationContext.handleSecondaryContextualization(DeserializationContext.java:867)
[info] at com.fasterxml.jackson.databind.DeserializationContext.findContextualValueDeserializer(DeserializationContext.java:623)
[info] at com.fasterxml.jackson.databind.deser.impl.PropertyBasedCreator.construct(PropertyBasedCreator.java:114)
[info] at com.fasterxml.jackson.databind.deser.BeanDeserializerBase.resolve(BeanDeserializerBase.java:636)
[info] at com.fasterxml.jackson.databind.deser.DeserializerCache._createAndCache2(DeserializerCache.java:294)
[info] at com.fasterxml.jackson.databind.deser.DeserializerCache._createAndCacheValueDeserializer(DeserializerCache.java:244)
[info] at com.fasterxml.jackson.databind.deser.DeserializerCache.findValueDeserializer(DeserializerCache.java:142)
[info] at com.fasterxml.jackson.databind.DeserializationContext.findRootValueDeserializer(DeserializationContext.java:654)
[info] at com.fasterxml.jackson.databind.ObjectMapper._findRootDeserializer(ObjectMapper.java:4956)
[info] at com.fasterxml.jackson.databind.ObjectMapper._readMapAndClose(ObjectMapper.java:4826)
[info] at com.fasterxml.jackson.databind.ObjectMapper.readValue(ObjectMapper.java:3772)
[info] at com.fasterxml.jackson.databind.ObjectMapper.readValue(ObjectMapper.java:3740)
[info] at org.apache.spark.util.JsonProtocol$.sparkEventFromJson(JsonProtocol.scala:877)
```
I updated to `@JsonIgnore`. Unfortunately, the constructor is no longer compatible, but the blast radius should be limited as most users of this API should be readers.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] HyukjinKwon commented on a diff in pull request #42550: [SPARK-44861][CONNECT] jsonignore SparkListenerConnectOperationStarted.planRequest
Posted by "HyukjinKwon (via GitHub)" <gi...@apache.org>.
HyukjinKwon commented on code in PR #42550:
URL: https://github.com/apache/spark/pull/42550#discussion_r1299526592
##########
connector/connect/server/src/main/scala/org/apache/spark/sql/connect/service/ExecuteEventsManager.scala:
##########
@@ -278,6 +278,7 @@ case class ExecuteEventsManager(executeHolder: ExecuteHolder, clock: Clock) {
* @param extraTags:
* Additional metadata during the request.
*/
+@JsonIgnoreProperties({ "planRequest" })
Review Comment:
I think this just works IIRC.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] jdesjean commented on a diff in pull request #42550: [SPARK-44861][CONNECT] jsonignore SparkListenerConnectOperationStarted.planRequest
Posted by "jdesjean (via GitHub)" <gi...@apache.org>.
jdesjean commented on code in PR #42550:
URL: https://github.com/apache/spark/pull/42550#discussion_r1300686813
##########
connector/connect/server/src/main/scala/org/apache/spark/sql/connect/service/ExecuteEventsManager.scala:
##########
@@ -278,6 +278,7 @@ case class ExecuteEventsManager(executeHolder: ExecuteHolder, clock: Clock) {
* @param extraTags:
* Additional metadata during the request.
*/
+@JsonIgnoreProperties({ "planRequest" })
Review Comment:
I added json serde testing as done in https://github.com/apache/spark/pull/23728. The test showed that `@JsonIgnoreProperties({ "planRequest" })` didn't work on read.
```
[info] - SPARK-43923: post started *** FAILED *** (2 seconds, 220 milliseconds)
[info] com.fasterxml.jackson.databind.exc.InvalidDefinitionException: Cannot find a deserializer for non-concrete Collection type [collection type; class com.google.protobuf.ProtocolStringList, contains [simple type, class java.lang.String]]
[info] at [Source: (String)"{"Event":"org.apache.spark.sql.connect.service.SparkListenerConnectOperationStarted","jobTag":"SparkConnect_OperationTag_User_7f8102fa-64a6-4c6e-a2a1-d701de9c0dc3_Session_1_Operation_6a771009-bc23-4767-bef7-6f75cf92b3a0","operationId":"6a771009-bc23-4767-bef7-6f75cf92b3a0","eventTime":0,"sessionId":"7f8102fa-64a6-4c6e-a2a1-d701de9c0dc3","userId":"1","userName":"userName","statementText":"limit {\n limit: 10\n}\n","sparkSessionTags":[],"extraTags":{}}"; line: 1, column: 1]
[info] at com.fasterxml.jackson.databind.exc.InvalidDefinitionException.from(InvalidDefinitionException.java:67)
[info] at com.fasterxml.jackson.databind.DeserializationContext.reportBadDefinition(DeserializationContext.java:1915)
[info] at com.fasterxml.jackson.databind.deser.DeserializerCache._createAndCache2(DeserializerCache.java:268)
[info] at com.fasterxml.jackson.databind.deser.DeserializerCache._createAndCacheValueDeserializer(DeserializerCache.java:244)
[info] at com.fasterxml.jackson.databind.deser.DeserializerCache.findValueDeserializer(DeserializerCache.java:142)
[info] at com.fasterxml.jackson.databind.DeserializationContext.findNonContextualValueDeserializer(DeserializationContext.java:644)
[info] at com.fasterxml.jackson.databind.deser.BeanDeserializerBase.resolve(BeanDeserializerBase.java:539)
[info] at com.fasterxml.jackson.databind.deser.DeserializerCache._createAndCache2(DeserializerCache.java:294)
[info] at com.fasterxml.jackson.databind.deser.DeserializerCache._createAndCacheValueDeserializer(DeserializerCache.java:244)
[info] at com.fasterxml.jackson.databind.deser.DeserializerCache.findValueDeserializer(DeserializerCache.java:142)
[info] at com.fasterxml.jackson.databind.DeserializationContext.findContextualValueDeserializer(DeserializationContext.java:621)
[info] at com.fasterxml.jackson.module.scala.deser.OptionDeserializer.createContextual(OptionDeserializerModule.scala:50)
[info] at com.fasterxml.jackson.databind.DeserializationContext.handleSecondaryContextualization(DeserializationContext.java:867)
[info] at com.fasterxml.jackson.databind.DeserializationContext.findContextualValueDeserializer(DeserializationContext.java:623)
[info] at com.fasterxml.jackson.databind.deser.impl.PropertyBasedCreator.construct(PropertyBasedCreator.java:114)
[info] at com.fasterxml.jackson.databind.deser.BeanDeserializerBase.resolve(BeanDeserializerBase.java:636)
[info] at com.fasterxml.jackson.databind.deser.DeserializerCache._createAndCache2(DeserializerCache.java:294)
[info] at com.fasterxml.jackson.databind.deser.DeserializerCache._createAndCacheValueDeserializer(DeserializerCache.java:244)
[info] at com.fasterxml.jackson.databind.deser.DeserializerCache.findValueDeserializer(DeserializerCache.java:142)
[info] at com.fasterxml.jackson.databind.DeserializationContext.findRootValueDeserializer(DeserializationContext.java:654)
[info] at com.fasterxml.jackson.databind.ObjectMapper._findRootDeserializer(ObjectMapper.java:4956)
[info] at com.fasterxml.jackson.databind.ObjectMapper._readMapAndClose(ObjectMapper.java:4826)
[info] at com.fasterxml.jackson.databind.ObjectMapper.readValue(ObjectMapper.java:3772)
[info] at com.fasterxml.jackson.databind.ObjectMapper.readValue(ObjectMapper.java:3740)
[info] at org.apache.spark.util.JsonProtocol$.sparkEventFromJson(JsonProtocol.scala:877)
```
I updated to `@JsonIgnore`. Unfortunately ,the constructor is no longer compatible, but the blast radius should be limited as most users of this API should be readers.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] rednaxelafx commented on pull request #42550: [SPARK-44861][CONNECT] jsonignore SparkListenerConnectOperationStarted.planRequest
Posted by "rednaxelafx (via GitHub)" <gi...@apache.org>.
rednaxelafx commented on PR #42550:
URL: https://github.com/apache/spark/pull/42550#issuecomment-1686757658
Interesting. For the newly added events for Spark Connect Server, we don't have a test suite of golden files for the expected event logs, so this PR's change didn't affect any golden file updates. It'd be be great if we can have test parity with the other events and have golden file based testing added. 🙏
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] jdesjean commented on a diff in pull request #42550: [SPARK-44861][CONNECT] jsonignore SparkListenerConnectOperationStarted.planRequest
Posted by "jdesjean (via GitHub)" <gi...@apache.org>.
jdesjean commented on code in PR #42550:
URL: https://github.com/apache/spark/pull/42550#discussion_r1300686813
##########
connector/connect/server/src/main/scala/org/apache/spark/sql/connect/service/ExecuteEventsManager.scala:
##########
@@ -278,6 +278,7 @@ case class ExecuteEventsManager(executeHolder: ExecuteHolder, clock: Clock) {
* @param extraTags:
* Additional metadata during the request.
*/
+@JsonIgnoreProperties({ "planRequest" })
Review Comment:
I added json serde testing as done in https://github.com/apache/spark/pull/23728 and`@JsonIgnoreProperties({ "planRequest" })` didn't work.
```
[info] - SPARK-43923: post started *** FAILED *** (2 seconds, 220 milliseconds)
[info] com.fasterxml.jackson.databind.exc.InvalidDefinitionException: Cannot find a deserializer for non-concrete Collection type [collection type; class com.google.protobuf.ProtocolStringList, contains [simple type, class java.lang.String]]
[info] at [Source: (String)"{"Event":"org.apache.spark.sql.connect.service.SparkListenerConnectOperationStarted","jobTag":"SparkConnect_OperationTag_User_7f8102fa-64a6-4c6e-a2a1-d701de9c0dc3_Session_1_Operation_6a771009-bc23-4767-bef7-6f75cf92b3a0","operationId":"6a771009-bc23-4767-bef7-6f75cf92b3a0","eventTime":0,"sessionId":"7f8102fa-64a6-4c6e-a2a1-d701de9c0dc3","userId":"1","userName":"userName","statementText":"limit {\n limit: 10\n}\n","sparkSessionTags":[],"extraTags":{}}"; line: 1, column: 1]
[info] at com.fasterxml.jackson.databind.exc.InvalidDefinitionException.from(InvalidDefinitionException.java:67)
[info] at com.fasterxml.jackson.databind.DeserializationContext.reportBadDefinition(DeserializationContext.java:1915)
[info] at com.fasterxml.jackson.databind.deser.DeserializerCache._createAndCache2(DeserializerCache.java:268)
[info] at com.fasterxml.jackson.databind.deser.DeserializerCache._createAndCacheValueDeserializer(DeserializerCache.java:244)
[info] at com.fasterxml.jackson.databind.deser.DeserializerCache.findValueDeserializer(DeserializerCache.java:142)
[info] at com.fasterxml.jackson.databind.DeserializationContext.findNonContextualValueDeserializer(DeserializationContext.java:644)
[info] at com.fasterxml.jackson.databind.deser.BeanDeserializerBase.resolve(BeanDeserializerBase.java:539)
[info] at com.fasterxml.jackson.databind.deser.DeserializerCache._createAndCache2(DeserializerCache.java:294)
[info] at com.fasterxml.jackson.databind.deser.DeserializerCache._createAndCacheValueDeserializer(DeserializerCache.java:244)
[info] at com.fasterxml.jackson.databind.deser.DeserializerCache.findValueDeserializer(DeserializerCache.java:142)
[info] at com.fasterxml.jackson.databind.DeserializationContext.findContextualValueDeserializer(DeserializationContext.java:621)
[info] at com.fasterxml.jackson.module.scala.deser.OptionDeserializer.createContextual(OptionDeserializerModule.scala:50)
[info] at com.fasterxml.jackson.databind.DeserializationContext.handleSecondaryContextualization(DeserializationContext.java:867)
[info] at com.fasterxml.jackson.databind.DeserializationContext.findContextualValueDeserializer(DeserializationContext.java:623)
[info] at com.fasterxml.jackson.databind.deser.impl.PropertyBasedCreator.construct(PropertyBasedCreator.java:114)
[info] at com.fasterxml.jackson.databind.deser.BeanDeserializerBase.resolve(BeanDeserializerBase.java:636)
[info] at com.fasterxml.jackson.databind.deser.DeserializerCache._createAndCache2(DeserializerCache.java:294)
[info] at com.fasterxml.jackson.databind.deser.DeserializerCache._createAndCacheValueDeserializer(DeserializerCache.java:244)
[info] at com.fasterxml.jackson.databind.deser.DeserializerCache.findValueDeserializer(DeserializerCache.java:142)
[info] at com.fasterxml.jackson.databind.DeserializationContext.findRootValueDeserializer(DeserializationContext.java:654)
[info] at com.fasterxml.jackson.databind.ObjectMapper._findRootDeserializer(ObjectMapper.java:4956)
[info] at com.fasterxml.jackson.databind.ObjectMapper._readMapAndClose(ObjectMapper.java:4826)
[info] at com.fasterxml.jackson.databind.ObjectMapper.readValue(ObjectMapper.java:3772)
[info] at com.fasterxml.jackson.databind.ObjectMapper.readValue(ObjectMapper.java:3740)
[info] at org.apache.spark.util.JsonProtocol$.sparkEventFromJson(JsonProtocol.scala:877)
```
I moved to `@JsonIgnore`. Unfortunately ,the constructor is no longer compatible, but the blast radius should be limited as most users of this API should be readers.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] jdesjean commented on a diff in pull request #42550: [SPARK-44861][CONNECT] jsonignore SparkListenerConnectOperationStarted.planRequest
Posted by "jdesjean (via GitHub)" <gi...@apache.org>.
jdesjean commented on code in PR #42550:
URL: https://github.com/apache/spark/pull/42550#discussion_r1300686813
##########
connector/connect/server/src/main/scala/org/apache/spark/sql/connect/service/ExecuteEventsManager.scala:
##########
@@ -278,6 +278,7 @@ case class ExecuteEventsManager(executeHolder: ExecuteHolder, clock: Clock) {
* @param extraTags:
* Additional metadata during the request.
*/
+@JsonIgnoreProperties({ "planRequest" })
Review Comment:
I added json serde testing as done in https://github.com/apache/spark/pull/23728 and`@JsonIgnoreProperties({ "planRequest" })` didn't work
```
[info] - SPARK-43923: post started *** FAILED *** (2 seconds, 220 milliseconds)
[info] com.fasterxml.jackson.databind.exc.InvalidDefinitionException: Cannot find a deserializer for non-concrete Collection type [collection type; class com.google.protobuf.ProtocolStringList, contains [simple type, class java.lang.String]]
[info] at [Source: (String)"{"Event":"org.apache.spark.sql.connect.service.SparkListenerConnectOperationStarted","jobTag":"SparkConnect_OperationTag_User_7f8102fa-64a6-4c6e-a2a1-d701de9c0dc3_Session_1_Operation_6a771009-bc23-4767-bef7-6f75cf92b3a0","operationId":"6a771009-bc23-4767-bef7-6f75cf92b3a0","eventTime":0,"sessionId":"7f8102fa-64a6-4c6e-a2a1-d701de9c0dc3","userId":"1","userName":"userName","statementText":"limit {\n limit: 10\n}\n","sparkSessionTags":[],"extraTags":{}}"; line: 1, column: 1]
[info] at com.fasterxml.jackson.databind.exc.InvalidDefinitionException.from(InvalidDefinitionException.java:67)
[info] at com.fasterxml.jackson.databind.DeserializationContext.reportBadDefinition(DeserializationContext.java:1915)
[info] at com.fasterxml.jackson.databind.deser.DeserializerCache._createAndCache2(DeserializerCache.java:268)
[info] at com.fasterxml.jackson.databind.deser.DeserializerCache._createAndCacheValueDeserializer(DeserializerCache.java:244)
[info] at com.fasterxml.jackson.databind.deser.DeserializerCache.findValueDeserializer(DeserializerCache.java:142)
[info] at com.fasterxml.jackson.databind.DeserializationContext.findNonContextualValueDeserializer(DeserializationContext.java:644)
[info] at com.fasterxml.jackson.databind.deser.BeanDeserializerBase.resolve(BeanDeserializerBase.java:539)
[info] at com.fasterxml.jackson.databind.deser.DeserializerCache._createAndCache2(DeserializerCache.java:294)
[info] at com.fasterxml.jackson.databind.deser.DeserializerCache._createAndCacheValueDeserializer(DeserializerCache.java:244)
[info] at com.fasterxml.jackson.databind.deser.DeserializerCache.findValueDeserializer(DeserializerCache.java:142)
[info] at com.fasterxml.jackson.databind.DeserializationContext.findContextualValueDeserializer(DeserializationContext.java:621)
[info] at com.fasterxml.jackson.module.scala.deser.OptionDeserializer.createContextual(OptionDeserializerModule.scala:50)
[info] at com.fasterxml.jackson.databind.DeserializationContext.handleSecondaryContextualization(DeserializationContext.java:867)
[info] at com.fasterxml.jackson.databind.DeserializationContext.findContextualValueDeserializer(DeserializationContext.java:623)
[info] at com.fasterxml.jackson.databind.deser.impl.PropertyBasedCreator.construct(PropertyBasedCreator.java:114)
[info] at com.fasterxml.jackson.databind.deser.BeanDeserializerBase.resolve(BeanDeserializerBase.java:636)
[info] at com.fasterxml.jackson.databind.deser.DeserializerCache._createAndCache2(DeserializerCache.java:294)
[info] at com.fasterxml.jackson.databind.deser.DeserializerCache._createAndCacheValueDeserializer(DeserializerCache.java:244)
[info] at com.fasterxml.jackson.databind.deser.DeserializerCache.findValueDeserializer(DeserializerCache.java:142)
[info] at com.fasterxml.jackson.databind.DeserializationContext.findRootValueDeserializer(DeserializationContext.java:654)
[info] at com.fasterxml.jackson.databind.ObjectMapper._findRootDeserializer(ObjectMapper.java:4956)
[info] at com.fasterxml.jackson.databind.ObjectMapper._readMapAndClose(ObjectMapper.java:4826)
[info] at com.fasterxml.jackson.databind.ObjectMapper.readValue(ObjectMapper.java:3772)
[info] at com.fasterxml.jackson.databind.ObjectMapper.readValue(ObjectMapper.java:3740)
[info] at org.apache.spark.util.JsonProtocol$.sparkEventFromJson(JsonProtocol.scala:877)
```
.
I moved to `@JsonIgnore`. Unfortunately ,the constructor is no longer compatible, but the blast radius should be limited as most users of this API should be readers.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] HyukjinKwon commented on a diff in pull request #42550: [SPARK-44861][CONNECT] jsonignore SparkListenerConnectOperationStarted.planRequest
Posted by "HyukjinKwon (via GitHub)" <gi...@apache.org>.
HyukjinKwon commented on code in PR #42550:
URL: https://github.com/apache/spark/pull/42550#discussion_r1299527368
##########
connector/connect/server/src/main/scala/org/apache/spark/sql/connect/service/ExecuteEventsManager.scala:
##########
@@ -278,6 +278,7 @@ case class ExecuteEventsManager(executeHolder: ExecuteHolder, clock: Clock) {
* @param extraTags:
* Additional metadata during the request.
*/
+@JsonIgnoreProperties({ "planRequest" })
Review Comment:
For testing, simply doing ser/de test should be enough, see also https://github.com/apache/spark/pull/23728 in which I fixed a similar problem.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] hvanhovell closed pull request #42550: [SPARK-44861][CONNECT] jsonignore SparkListenerConnectOperationStarted.planRequest
Posted by "hvanhovell (via GitHub)" <gi...@apache.org>.
hvanhovell closed pull request #42550: [SPARK-44861][CONNECT] jsonignore SparkListenerConnectOperationStarted.planRequest
URL: https://github.com/apache/spark/pull/42550
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] jdesjean commented on a diff in pull request #42550: [SPARK-44861][CONNECT] jsonignore SparkListenerConnectOperationStarted.planRequest
Posted by "jdesjean (via GitHub)" <gi...@apache.org>.
jdesjean commented on code in PR #42550:
URL: https://github.com/apache/spark/pull/42550#discussion_r1300686813
##########
connector/connect/server/src/main/scala/org/apache/spark/sql/connect/service/ExecuteEventsManager.scala:
##########
@@ -278,6 +278,7 @@ case class ExecuteEventsManager(executeHolder: ExecuteHolder, clock: Clock) {
* @param extraTags:
* Additional metadata during the request.
*/
+@JsonIgnoreProperties({ "planRequest" })
Review Comment:
I added json serde testing as done in https://github.com/apache/spark/pull/23728 and`. The test showed that @JsonIgnoreProperties({ "planRequest" })` didn't work.
```
[info] - SPARK-43923: post started *** FAILED *** (2 seconds, 220 milliseconds)
[info] com.fasterxml.jackson.databind.exc.InvalidDefinitionException: Cannot find a deserializer for non-concrete Collection type [collection type; class com.google.protobuf.ProtocolStringList, contains [simple type, class java.lang.String]]
[info] at [Source: (String)"{"Event":"org.apache.spark.sql.connect.service.SparkListenerConnectOperationStarted","jobTag":"SparkConnect_OperationTag_User_7f8102fa-64a6-4c6e-a2a1-d701de9c0dc3_Session_1_Operation_6a771009-bc23-4767-bef7-6f75cf92b3a0","operationId":"6a771009-bc23-4767-bef7-6f75cf92b3a0","eventTime":0,"sessionId":"7f8102fa-64a6-4c6e-a2a1-d701de9c0dc3","userId":"1","userName":"userName","statementText":"limit {\n limit: 10\n}\n","sparkSessionTags":[],"extraTags":{}}"; line: 1, column: 1]
[info] at com.fasterxml.jackson.databind.exc.InvalidDefinitionException.from(InvalidDefinitionException.java:67)
[info] at com.fasterxml.jackson.databind.DeserializationContext.reportBadDefinition(DeserializationContext.java:1915)
[info] at com.fasterxml.jackson.databind.deser.DeserializerCache._createAndCache2(DeserializerCache.java:268)
[info] at com.fasterxml.jackson.databind.deser.DeserializerCache._createAndCacheValueDeserializer(DeserializerCache.java:244)
[info] at com.fasterxml.jackson.databind.deser.DeserializerCache.findValueDeserializer(DeserializerCache.java:142)
[info] at com.fasterxml.jackson.databind.DeserializationContext.findNonContextualValueDeserializer(DeserializationContext.java:644)
[info] at com.fasterxml.jackson.databind.deser.BeanDeserializerBase.resolve(BeanDeserializerBase.java:539)
[info] at com.fasterxml.jackson.databind.deser.DeserializerCache._createAndCache2(DeserializerCache.java:294)
[info] at com.fasterxml.jackson.databind.deser.DeserializerCache._createAndCacheValueDeserializer(DeserializerCache.java:244)
[info] at com.fasterxml.jackson.databind.deser.DeserializerCache.findValueDeserializer(DeserializerCache.java:142)
[info] at com.fasterxml.jackson.databind.DeserializationContext.findContextualValueDeserializer(DeserializationContext.java:621)
[info] at com.fasterxml.jackson.module.scala.deser.OptionDeserializer.createContextual(OptionDeserializerModule.scala:50)
[info] at com.fasterxml.jackson.databind.DeserializationContext.handleSecondaryContextualization(DeserializationContext.java:867)
[info] at com.fasterxml.jackson.databind.DeserializationContext.findContextualValueDeserializer(DeserializationContext.java:623)
[info] at com.fasterxml.jackson.databind.deser.impl.PropertyBasedCreator.construct(PropertyBasedCreator.java:114)
[info] at com.fasterxml.jackson.databind.deser.BeanDeserializerBase.resolve(BeanDeserializerBase.java:636)
[info] at com.fasterxml.jackson.databind.deser.DeserializerCache._createAndCache2(DeserializerCache.java:294)
[info] at com.fasterxml.jackson.databind.deser.DeserializerCache._createAndCacheValueDeserializer(DeserializerCache.java:244)
[info] at com.fasterxml.jackson.databind.deser.DeserializerCache.findValueDeserializer(DeserializerCache.java:142)
[info] at com.fasterxml.jackson.databind.DeserializationContext.findRootValueDeserializer(DeserializationContext.java:654)
[info] at com.fasterxml.jackson.databind.ObjectMapper._findRootDeserializer(ObjectMapper.java:4956)
[info] at com.fasterxml.jackson.databind.ObjectMapper._readMapAndClose(ObjectMapper.java:4826)
[info] at com.fasterxml.jackson.databind.ObjectMapper.readValue(ObjectMapper.java:3772)
[info] at com.fasterxml.jackson.databind.ObjectMapper.readValue(ObjectMapper.java:3740)
[info] at org.apache.spark.util.JsonProtocol$.sparkEventFromJson(JsonProtocol.scala:877)
```
I moved to `@JsonIgnore`. Unfortunately ,the constructor is no longer compatible, but the blast radius should be limited as most users of this API should be readers.
##########
connector/connect/server/src/main/scala/org/apache/spark/sql/connect/service/ExecuteEventsManager.scala:
##########
@@ -278,6 +278,7 @@ case class ExecuteEventsManager(executeHolder: ExecuteHolder, clock: Clock) {
* @param extraTags:
* Additional metadata during the request.
*/
+@JsonIgnoreProperties({ "planRequest" })
Review Comment:
I added json serde testing as done in https://github.com/apache/spark/pull/23728 and`. The test showed that @JsonIgnoreProperties({ "planRequest" })` didn't work.
```
[info] - SPARK-43923: post started *** FAILED *** (2 seconds, 220 milliseconds)
[info] com.fasterxml.jackson.databind.exc.InvalidDefinitionException: Cannot find a deserializer for non-concrete Collection type [collection type; class com.google.protobuf.ProtocolStringList, contains [simple type, class java.lang.String]]
[info] at [Source: (String)"{"Event":"org.apache.spark.sql.connect.service.SparkListenerConnectOperationStarted","jobTag":"SparkConnect_OperationTag_User_7f8102fa-64a6-4c6e-a2a1-d701de9c0dc3_Session_1_Operation_6a771009-bc23-4767-bef7-6f75cf92b3a0","operationId":"6a771009-bc23-4767-bef7-6f75cf92b3a0","eventTime":0,"sessionId":"7f8102fa-64a6-4c6e-a2a1-d701de9c0dc3","userId":"1","userName":"userName","statementText":"limit {\n limit: 10\n}\n","sparkSessionTags":[],"extraTags":{}}"; line: 1, column: 1]
[info] at com.fasterxml.jackson.databind.exc.InvalidDefinitionException.from(InvalidDefinitionException.java:67)
[info] at com.fasterxml.jackson.databind.DeserializationContext.reportBadDefinition(DeserializationContext.java:1915)
[info] at com.fasterxml.jackson.databind.deser.DeserializerCache._createAndCache2(DeserializerCache.java:268)
[info] at com.fasterxml.jackson.databind.deser.DeserializerCache._createAndCacheValueDeserializer(DeserializerCache.java:244)
[info] at com.fasterxml.jackson.databind.deser.DeserializerCache.findValueDeserializer(DeserializerCache.java:142)
[info] at com.fasterxml.jackson.databind.DeserializationContext.findNonContextualValueDeserializer(DeserializationContext.java:644)
[info] at com.fasterxml.jackson.databind.deser.BeanDeserializerBase.resolve(BeanDeserializerBase.java:539)
[info] at com.fasterxml.jackson.databind.deser.DeserializerCache._createAndCache2(DeserializerCache.java:294)
[info] at com.fasterxml.jackson.databind.deser.DeserializerCache._createAndCacheValueDeserializer(DeserializerCache.java:244)
[info] at com.fasterxml.jackson.databind.deser.DeserializerCache.findValueDeserializer(DeserializerCache.java:142)
[info] at com.fasterxml.jackson.databind.DeserializationContext.findContextualValueDeserializer(DeserializationContext.java:621)
[info] at com.fasterxml.jackson.module.scala.deser.OptionDeserializer.createContextual(OptionDeserializerModule.scala:50)
[info] at com.fasterxml.jackson.databind.DeserializationContext.handleSecondaryContextualization(DeserializationContext.java:867)
[info] at com.fasterxml.jackson.databind.DeserializationContext.findContextualValueDeserializer(DeserializationContext.java:623)
[info] at com.fasterxml.jackson.databind.deser.impl.PropertyBasedCreator.construct(PropertyBasedCreator.java:114)
[info] at com.fasterxml.jackson.databind.deser.BeanDeserializerBase.resolve(BeanDeserializerBase.java:636)
[info] at com.fasterxml.jackson.databind.deser.DeserializerCache._createAndCache2(DeserializerCache.java:294)
[info] at com.fasterxml.jackson.databind.deser.DeserializerCache._createAndCacheValueDeserializer(DeserializerCache.java:244)
[info] at com.fasterxml.jackson.databind.deser.DeserializerCache.findValueDeserializer(DeserializerCache.java:142)
[info] at com.fasterxml.jackson.databind.DeserializationContext.findRootValueDeserializer(DeserializationContext.java:654)
[info] at com.fasterxml.jackson.databind.ObjectMapper._findRootDeserializer(ObjectMapper.java:4956)
[info] at com.fasterxml.jackson.databind.ObjectMapper._readMapAndClose(ObjectMapper.java:4826)
[info] at com.fasterxml.jackson.databind.ObjectMapper.readValue(ObjectMapper.java:3772)
[info] at com.fasterxml.jackson.databind.ObjectMapper.readValue(ObjectMapper.java:3740)
[info] at org.apache.spark.util.JsonProtocol$.sparkEventFromJson(JsonProtocol.scala:877)
```
I updated to `@JsonIgnore`. Unfortunately ,the constructor is no longer compatible, but the blast radius should be limited as most users of this API should be readers.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] hvanhovell commented on pull request #42550: [SPARK-44861][CONNECT] jsonignore SparkListenerConnectOperationStarted.planRequest
Posted by "hvanhovell (via GitHub)" <gi...@apache.org>.
hvanhovell commented on PR #42550:
URL: https://github.com/apache/spark/pull/42550#issuecomment-1690284866
Merging!
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org