Uploaded image for project: 'Data Management'
  1. Data Management
  2. DM-33052

Update SAL/Kafka producer to support schema evolution

    XMLWordPrintable

    Details

    • Type: Story
    • Status: Done
    • Resolution: Done
    • Fix Version/s: None
    • Component/s: None
    • Labels:
    • Story Points:
      2
    • Sprint:
      TSSW Sprint - Apr 11 - Apr 25
    • Team:
      Telescope and Site
    • Urgent?:
      No

      Description

      It turns out we can easily support schema evolution in Kafka/Avro by specifying a default value for each field in the schema created by the SAL/Kafka producers.

      This is a simple change. The main question is whether the default float value should be 0 (like DDS) or NaN. I doubt it makes much difference, because by the time the SAL/Kafka producer sees the SAL data, all fields that are in the current schema have been filled in. Thus this default only affects fields that we remove from the XML. I am inclined to use 0 because it appears NaN support in Avro is somewhat iffy and it matches OpenSplice.

        Attachments

          Issue Links

            Activity

            Hide
            afausti Angelo Fausti added a comment -

            For the default values, I would recommend using NaN for float and arrays of float for the reasons discussed in https://sqr-053.lsst.io/#recommendations. 0 is not a good representation for a missing value, NaN works better with InfluxDB and Parquet and correctly represents missing values when you return a Pandas data frame.

            Show
            afausti Angelo Fausti added a comment - For the default values, I would recommend using NaN for float and arrays of float for the reasons discussed in  https://sqr-053.lsst.io/#recommendations.  0 is not a good representation for a missing value, NaN works better with InfluxDB and Parquet and correctly represents missing values when you return a Pandas data frame.
            Hide
            rowen Russell Owen added a comment - - edited

            I agree that nan would be nice in principle, but I am strongly inclined to use 0 because that is what Kafka and DDS both use. I fear it will result in hard-to-diagnose errors if we don't match the standard messaging systems. Using 0 means that fields we no longer write are treated the same as fields that we don't specify.

            Show
            rowen Russell Owen added a comment - - edited I agree that nan would be nice in principle, but I am strongly inclined to use 0 because that is what Kafka and DDS both use. I fear it will result in hard-to-diagnose errors if we don't match the standard messaging systems. Using 0 means that fields we no longer write are treated the same as fields that we don't specify.
            Hide
            rowen Russell Owen added a comment - - edited

            To test this I ran kafka-aggregator and did the following (this is based on the testing Angelo did in DM-30082, but simplified):

            First I the changed the code so that private_efdStamp and private_kafkaStamp had no default value. Then:

            • Run a producer for the Test SAL component to register the standard schema.
            • Modify this code to remove private_kafkaStamp. The new schema was rejected because private_kafkaStamp had no default value.
            • Undo that change and add a garbage field with no default. The new schema was rejected.

            Fix the code to specify default=0 for private_efdStamp and private_kafkaStamp (the two fields salkafka adds to each topic). Restart the Kafka servers. Run the following tests:

            • Run with standard schema.
            • Delete the private_efdStamp field and run. The new schema was accepted.
            • Add a garbage field with a default value. The new schema was accepted.

            I also tried to read the lsst.sal.Test.logevent_heartbeat topic in all cases, but no data arrived, even with the develop branch, so I was clearly doing something wrong. However, I feel the tests above are fairly convincing.

            Pull request: https://github.com/lsst-ts/ts_salkafka/pull/25

            Show
            rowen Russell Owen added a comment - - edited To test this I ran kafka-aggregator and did the following (this is based on the testing Angelo did in DM-30082 , but simplified): First I the changed the code so that private_efdStamp and private_kafkaStamp had no default value. Then: Run a producer for the Test SAL component to register the standard schema. Modify this code to remove private_kafkaStamp. The new schema was rejected because private_kafkaStamp had no default value. Undo that change and add a garbage field with no default. The new schema was rejected. Fix the code to specify default=0 for private_efdStamp and private_kafkaStamp (the two fields salkafka adds to each topic). Restart the Kafka servers. Run the following tests: Run with standard schema. Delete the private_efdStamp field and run. The new schema was accepted. Add a garbage field with a default value. The new schema was accepted. I also tried to read the lsst.sal.Test.logevent_heartbeat topic in all cases, but no data arrived, even with the develop branch, so I was clearly doing something wrong. However, I feel the tests above are fairly convincing. Pull request: https://github.com/lsst-ts/ts_salkafka/pull/25
            Hide
            afausti Angelo Fausti added a comment -

            Adding a default value for fields in the Avro schema definitely helps with schema evolution. Ideally, the default value should be NaN as proposed in SQR-053, but the DDS version we use does not support the @default annotation. The convention is to use 0 in DDS. Note that using 0 as the default value in the Avro messages will write 0's all the way to InfluxDB and Pandas when the data is retrieved (instead of NaN). I don’t think we can do better at this moment and the advantage I see is that we are using the same convention in both DDS and Kafka.

            This is related to RFC-763 which we might still consider once our DDS version support the @default annotation.

            Show
            afausti Angelo Fausti added a comment - Adding a default value for fields in the Avro schema definitely helps with schema evolution. Ideally, the default value should be NaN as proposed in SQR-053, but the DDS version we use does not support the @default annotation. The convention is to use 0 in DDS. Note that using 0 as the default value in the Avro messages will write 0's all the way to InfluxDB and Pandas when the data is retrieved (instead of NaN). I don’t think we can do better at this moment and the advantage I see is that we are using the same convention in both DDS and Kafka. This is related to RFC-763 which we might still consider once our DDS version support the @default annotation.
            Hide
            rowen Russell Owen added a comment -

            Merged to develop. I'll hold off release for now since cycle 25 is frozen.

            Show
            rowen Russell Owen added a comment - Merged to develop. I'll hold off release for now since cycle 25 is frozen.

              People

              Assignee:
              rowen Russell Owen
              Reporter:
              rowen Russell Owen
              Reviewers:
              Angelo Fausti
              Watchers:
              Angelo Fausti, Russell Owen, Tiago Ribeiro
              Votes:
              0 Vote for this issue
              Watchers:
              3 Start watching this issue

                Dates

                Created:
                Updated:
                Resolved:

                  Jenkins

                  No builds found.